Skip to content

Commit

Permalink
tapback: Remove physical-device support
Browse files Browse the repository at this point in the history 
Moving to an NBD-only approach, physical-device is getting populated
with a major:minor of 0:0.  That is because we want to place the nbd
unix socket path, /run/blktap-control/nbd$pid.$minor, in
physical-device-path.  The block scripts use stat -L -c %t:%T "$1",
which only works on block or char device nodes.  Running it against the
unix socket returns 0:0.

physical-device-path has everything we want - the usable nbd server
path, pid and "minor".  Rely on that being set to setup the tapdisk
connection.

Dropping physical-device also avoids the issue that looking up by
"minor" would be incorrect since it would always be zero.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
  • Loading branch information
@jandryuk
jandryuk committed Apr 28, 2023
1 parent 09de75e commit e443937
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 185 deletions.
187 changes: 9 additions & 178 deletions tapback/backend.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ physical_device_path_changed(vbd_t *device) {

/*
* The front-end might have switched to state Connected before
* physical-device is written. Check it's state and connect if necessary.
* physical-device-path is written. Check it's state and connect if
* necessary.
*
* TODO blkback ignores connection errors, let's do the same until we
* know better.
Expand All @@ -382,162 +383,6 @@ physical_device_path_changed(vbd_t *device) {
return err;
}

/**
* Retrieves the minor number and locates the corresponding tapdisk, storing
* all relevant information in @device. Then, it attempts to advance the Xenbus
* state as it might be that everything is ready and all that was missing was
* the physical device.
*
* We might already have read the major/minor, but if the physical-device
* key triggered we need to make sure it hasn't changed. This also protects us
* against restarted transactions.
*
* Returns 0 on success, a negative error code otherwise.
*/
static int
physical_device_changed(vbd_t *device) {

char * s = NULL, *p = NULL, *end = NULL;
int err = 0, major = 0, minor = 0;
unsigned int info;

ASSERT(device);

INFO(device, "physical_device_changed\n");

/*
* Get the minor.
*/
s = tapback_device_read(device, XBT_NULL, PHYS_DEV_KEY);
if (!s) {
err = -errno;
if (err != -ENOENT)
WARN(device, "failed to read the physical-device: %s\n",
strerror(-err));
goto out;
}

if (strlen(s) == 0) {
/* empty string is a missing device */
err = -ENOENT;
goto out;
}

/*
* The XenStore key physical-device contains "major:minor" in hex.
*/
p = strtok(s, ":");
if (!p) {
WARN(device, "malformed physical device '%s'\n", s);
err = -EINVAL;
goto out;
}
major = strtol(p, &end, 16);
if (*end != 0 || end == p) {
WARN(device, "malformed physical device '%s'\n", s);
err = -EINVAL;
goto out;
}
p = strtok(NULL, ":");
if (unlikely(!p)) {
WARN(device, "malformed physical device '%s'\n", s);
err = -EINVAL;
goto out;
}
minor = strtol(p, &end, 16);
if (*end != 0 || end == p) {
WARN(device, "malformed physical device '%s'\n", s);
err = -EINVAL;
goto out;
}

if ((device->major >= 0 || device->minor >= 0) &&
(major != device->major || minor != device->minor)) {
WARN(device, "changing physical device from %x:%x to %x:%x not "
"supported\n", device->major, device->minor, major, minor);
err = -ENOSYS;
goto out;
}

if (major != tapdev_major) {
WARN(device, "ignoring non-blktap2 physical device: %d\n", major);
err = -EINVAL;
goto out;
}

device->major = major;
device->minor = minor;

device->tap = malloc(sizeof(*device->tap));
if (!device->tap) {
err = -ENOMEM;
goto out;
}

/*
* XXX If the physical-device key has been written we expect a tapdisk to
* have been created. If tapdisk is created after the physical-device key
* is written we have no way of being notified, so we will not be able to
* advance the back-end state.
*/

DBG(device, "need to find tapdisk serving minor=%d\n", device->minor);

err = find_tapdisk(device->minor, device->tap);
if (err) {
WARN(device, "error looking for tapdisk: %s\n", strerror(-err));
goto out;
}

INFO(device, "found tapdisk[%d], for %d:%d\n", device->tap->pid, device->major, device->minor);

/*
* get the VBD parameters from the tapdisk
*/
if ((err = tap_ctl_info(device->tap->pid, &device->sectors,
&device->sector_size, &info,
device->minor))) {
WARN(device, "error retrieving disk characteristics: %s\n",
strerror(-err));
goto out;
}

err = tapback_device_printf(device, XBT_NULL, "kthread-pid", false, "%d",
device->tap->pid);
if (unlikely(err)) {
WARN(device, "warning: failed to write kthread-pid: %s\n",
strerror(-err));
goto out;
}

if (device->sector_size & 0x1ff || device->sectors <= 0) {
WARN(device, "warning: unexpected device characteristics: sector "
"size=%d, sectors=%llu\n", device->sector_size,
device->sectors);
}

/*
* The front-end might have switched to state Connected before
* physical-device is written. Check it's state and connect if necessary.
*
* TODO blkback ignores connection errors, let's do the same until we
* know better.
*/
err = -frontend_changed(device, device->frontend_state);
if (err)
WARN(device, "failed to switch state: %s (error ignored)\n",
strerror(-err));
err = 0;
out:
if (err) {
free(device->tap);
device->tap = NULL;
device->sector_size = device->sectors = device->info = 0;
}
free(s);
return err;
}

/**
* Start watching the front-end state path.
*/
Expand Down Expand Up @@ -607,9 +452,9 @@ frontend(vbd_t *device) {
* FIXME This function REQUIRES the device->frontend_path member to be
* populated, and this is done by frontend().
*
* Connecting to the front-end requires the physical-device key to have been
* written. This function will attempt to connect anyway, and connecting will
* fail half-way through. This is expected.
* Connecting to the front-end requires the physical-device-path key to have
* been written. This function will attempt to connect anyway, and connecting
* will fail half-way through. This is expected.
*
* Returns 0 in success, -errno on failure.
*/
Expand Down Expand Up @@ -697,7 +542,7 @@ hotplug_status_changed(vbd_t * const device) {
* Ignore connection errors as the front-end might not yet be
* ready. blkback doesn't wait for this XenStore key to be written,
* so we choose to handle this the same way we do with
* physical-device.
* physical-device-path.
*/
INFO(device, "failed to connect to the front-end: %s "
"(error ignored)\n", strerror(err));
Expand Down Expand Up @@ -757,18 +602,6 @@ reconnect(vbd_t *device) {
goto out;
}

err = physical_device_changed(device);
if (err) {
if (err == -ENOENT) {
DBG(device, "no physical device yet\n");
err = 0;
} else {
WARN(device, "failed to retrieve physical device information: "
"%s\n", strerror(-err));
goto out;
}
}

err = hotplug_status_changed(device);
if (err) {
if (err == -ENOENT) {
Expand Down Expand Up @@ -892,12 +725,10 @@ tapback_backend_probe_device(backend_t *backend,
* TODO Replace this with a despatch table mapping XenStore keys to
* callbacks.
*
* XXX physical_device_changed() and hotplug_status_changed() require
* frontend() to have been called beforehand. This is achieved by
* calling reconnect by calling reconnect() when the VBD is created.
* XXX hotplug_status_changed() require frontend() to have been called
* beforehand. This is achieved by calling reconnect() when the VBD is
* created.
*/
if (!strcmp(PHYS_DEV_KEY, comp))
err = physical_device_changed(device);
if (!strcmp(PHYS_DEV_PATH_KEY, comp))
err = physical_device_path_changed(device);
else if (!strcmp(FRONTEND_KEY, comp))
Expand Down
9 changes: 3 additions & 6 deletions tapback/frontend.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ connect_tap(vbd_t * const device)
*/
ASSERT(!device->connected);

/*
* The physical-device XenStore key has not been written yet.
*/
if (!device->tap) {
DBG(device, "no tapdisk yet\n");
err = ESRCH;
Expand Down Expand Up @@ -349,8 +346,8 @@ connect_frontend(vbd_t *device) {
/*
* Returns 0 on success, a positive error code otherwise.
*
* If tapdisk is not yet available (the physical-device key has not yet been
* written), ESRCH is returned.
* If tapdisk is not yet available (the physical-device-path key has not yet
* been written), ESRCH is returned.
*/
static inline int
xenbus_connect(vbd_t *device) {
Expand All @@ -360,7 +357,7 @@ xenbus_connect(vbd_t *device) {

err = connect_tap(device);
/*
* No tapdisk yet (the physical-device XenStore key has not been written).
* No tapdisk yet.
*/
if (err == ESRCH)
goto out;
Expand Down
1 change: 0 additions & 1 deletion tapback/tapback.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ int pretty_time(char *buf, unsigned char buf_len);
* Pre-defined XenStore path components used for running the XenBus protocol.
*/
#define XENSTORE_BACKEND "backend"
#define PHYS_DEV_KEY "physical-device"
#define PHYS_DEV_PATH_KEY "physical-device-path"
#define HOTPLUG_STATUS_KEY "hotplug-status"
#define MODE_KEY "mode"
Expand Down

0 comments on commit e443937

Please sign in to comment.