[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl: locally attach disks with hotplug scripts for bootloader execution
El 25/06/15 a les 19.22, George Dunlap ha escrit: > On 06/25/2015 03:17 PM, Roger Pau Monne wrote: >> > Or else bootloader execution fails. Tested using an iSCSI disk. >> > >> > Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx> >> > Reported-by: Hildebrand, Nils <Nils.Hildebrand@xxxxxxxxxxx> >> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> >> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx> >> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx> >> > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx> >> > --- >> > tools/libxl/libxl.c | 17 ++++++++++++----- >> > 1 file changed, 12 insertions(+), 5 deletions(-) >> > >> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >> > index 9117b01..6430836 100644 >> > --- a/tools/libxl/libxl.c >> > +++ b/tools/libxl/libxl.c >> > @@ -3063,9 +3063,16 @@ void >> > libxl__device_disk_local_initiate_attach(libxl__egc *egc, >> > >> > switch (disk->backend) { >> > case LIBXL_DISK_BACKEND_PHY: >> > - LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk >> > %s", >> > - disk->pdev_path); >> > - dev = disk->pdev_path; >> > + if (disk->script == NULL && disk->backend_domname == NULL) { >> > + LOG(DEBUG, "locally attaching PHY disk %s", >> > disk->pdev_path); >> > + dev = disk->pdev_path; >> > + } else { >> > + libxl__prepare_ao_device(ao, &dls->aodev); >> > + dls->aodev.callback = local_device_attach_cb; >> > + device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, >> > &dls->aodev, >> > + libxl__alloc_vdev, (void *) blkdev_start); >> > + return; >> > + } > Although having said that -- isn't there also a bug here wrt qdisk > backends in a driver domain not being attached? > > Could we do something like the attached patch? > > This will do a local attach for blktap as well, which isn't strictly > necessary, but should work pretty much the same as for the block > scripts. (And anyway I'm about to replace the blktap stuff with block > scripts anyway.) > The logic in the patch looks fine to me, if you can assert that blktap is also capable of local-attach. > > > 0001-libxl-Make-local_initiate_attach-more-rational.patch > > >>From c4584a7a61e047bb87fb6133116c35dc0e79ab6d Mon Sep 17 00:00:00 2001 > From: George Dunlap <george.dunlap@xxxxxxxxxxxxx> > Date: Thu, 25 Jun 2015 18:11:21 +0100 > Subject: [PATCH] libxl: Make local_initiate_attach more rational > > There are a lot of paths through > libxl__device_disk_local_initiate_attach(), but they all really boil > down to one thing: Can we just access the file directly, or do we need > to attach it? > > The requirements for direct access are fairly simple: > * Is this local (as opposed to a driver domain)? > * Is this a raw format (as opposed to cooked)? > * Does this have no scripts associated with it? > > If it meets all those requirements, we can access it directly; > otherwise we need to attach it. > > This fixes a bug where bootloader execution fails for disks with > hotplug scripts. > > This should fix a theoretical bug when using a qdisk backend in a > driver domain. (Not tested.) > > Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx> > --- > tools/libxl/libxl.c | 89 > +++++++++++++++++------------------------------------ > 1 file changed, 28 insertions(+), 61 deletions(-) > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index d86ea62..8e795e3 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -3073,45 +3073,8 @@ void > libxl__device_disk_local_initiate_attach(libxl__egc *egc, > > switch (disk->backend) { > case LIBXL_DISK_BACKEND_PHY: > - LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk > %s", > - disk->pdev_path); > - dev = disk->pdev_path; > - break; > case LIBXL_DISK_BACKEND_TAP: > - switch (disk->format) { > - case LIBXL_DISK_FORMAT_RAW: > - /* optimise away the early tapdisk attach in this case */ > - LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching" > - " tap disk %s directly (ie without using blktap)", > - disk->pdev_path); > - dev = disk->pdev_path; > - break; > - case LIBXL_DISK_FORMAT_VHD: > - dev = libxl__blktap_devpath(gc, disk->pdev_path, > - disk->format); > - break; > - case LIBXL_DISK_FORMAT_QCOW: > - case LIBXL_DISK_FORMAT_QCOW2: > - abort(); /* prevented by libxl__device_disk_set_backend */ > - default: > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, > - "unrecognized disk format: %d", disk->format); > - rc = ERROR_FAIL; > - goto out; > - } > - break; > case LIBXL_DISK_BACKEND_QDISK: > - if (disk->format != LIBXL_DISK_FORMAT_RAW) { > - libxl__prepare_ao_device(ao, &dls->aodev); > - dls->aodev.callback = local_device_attach_cb; > - device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, > - &dls->aodev, libxl__alloc_vdev, > - (void *) blkdev_start); > - return; > - } else { > - dev = disk->pdev_path; > - } > - LOG(DEBUG, "locally attaching qdisk %s", dev); > break; > default: > LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend " Do we need to keep the whole switch statement just for the default (error) case? Wouldn't it be better to just replace it with an if condition? Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |