From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] libxl: locally attach disks with hotplug scripts for bootloader execution Date: Thu, 25 Jun 2015 18:22:54 +0100 Message-ID: <558C38EE.80103@eu.citrix.com> References: <1435241865-82765-1-git-send-email-roger.pau@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050002040902070707020203" Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z8As5-0001Un-4G for xen-devel@lists.xenproject.org; Thu, 25 Jun 2015 17:23:29 +0000 In-Reply-To: <1435241865-82765-1-git-send-email-roger.pau@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Roger Pau Monne , xen-devel@lists.xenproject.org Cc: Wei Liu , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org --------------050002040902070707020203 Content-Type: text/plain; charset="utf-8" Content-Length: 2013 Content-Transfer-Encoding: quoted-printable 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=C3=A9 > Reported-by: Hildebrand, Nils > Cc: Ian Jackson > Cc: Ian Campbell > Cc: Wei Liu > Cc: George Dunlap > --- > 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 =3D disk->pdev_path; > + if (disk->script =3D=3D NULL && disk->backend_domname =3D=3D NULL) { > + LOG(DEBUG, "locally attaching PHY disk %s", disk->pdev_path); > + dev =3D disk->pdev_path; > + } else { > + libxl__prepare_ao_device(ao, &dls->aodev); > + dls->aodev.callback =3D 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=3F Could we do something like the attached patch=3F 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.) -George --------------050002040902070707020203 Content-Type: text/x-patch; name="0001-libxl-Make-local_initiate_attach-more-rational.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-libxl-Make-local_initiate_attach-more-rational.patch" =46rom c4584a7a61e047bb87fb6133116c35dc0e79ab6d Mon Sep 17 00:00:00 2001 From: George Dunlap 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 --- 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(libx= l__egc *egc, =20 switch (disk->backend) { case LIBXL_DISK_BACKEND_PHY: - LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY dis= k %s", - disk->pdev_path); - dev =3D 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 blkt= ap)", - disk->pdev_path); - dev =3D disk->pdev_path; - break; - case LIBXL_DISK_FORMAT_VHD: - dev =3D 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 =3D ERROR_FAIL; - goto out; - } - break; case LIBXL_DISK_BACKEND_QDISK: - if (disk->format !=3D LIBXL_DISK_FORMAT_RAW) { - libxl__prepare_ao_device(ao, &dls->aodev); - dls->aodev.callback =3D local_device_attach_cb; - device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, - &dls->aodev, libxl__alloc_vdev, - (void *) blkdev_start); - return; - } else { - dev =3D disk->pdev_path; - } - LOG(DEBUG, "locally attaching qdisk %s", dev); break; default: LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend= " @@ -3120,6 +3083,21 @@ void libxl__device_disk_local_initiate_attach(libx= l__egc *egc, goto out; } =20 + /* If this is in a driver domain, or it's not a raw format, or it in= volves + * running a script, we have to do a local attach. */ + if (disk->backend_domname !=3D NULL + || disk->format !=3D LIBXL_DISK_FORMAT_RAW + || disk->script !=3D NULL) { + libxl__prepare_ao_device(ao, &dls->aodev); + dls->aodev.callback =3D local_device_attach_cb; + device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev, + libxl__alloc_vdev, (void *) blkdev_start); + return; + } + + LOG(DEBUG, "locally attaching RAW disk %s", disk->pdev_path); + dev =3D disk->pdev_path; + if (dev !=3D NULL) dls->diskpath =3D libxl__strdup(gc, dev); =20 @@ -3152,7 +3130,7 @@ static void local_device_attach_cb(libxl__egc *egc,= libxl__ao_device *aodev) } =20 dev =3D GCSPRINTF("/dev/%s", disk->vdev); - LOG(DEBUG, "locally attaching qdisk %s", dev); + LOG(DEBUG, "locally attaching disk %s", dev); =20 rc =3D libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &dev= ice); if (rc < 0) @@ -3191,29 +3169,18 @@ void libxl__device_disk_local_initiate_detach(lib= xl__egc *egc, =20 if (!dls->diskpath) goto out; =20 - switch (disk->backend) { - case LIBXL_DISK_BACKEND_QDISK: - if (disk->vdev !=3D NULL) { - GCNEW(device); - rc =3D libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID= , - disk, device); - if (rc !=3D 0) goto out; - - aodev->action =3D LIBXL__DEVICE_ACTION_REMOVE; - aodev->dev =3D device; - aodev->callback =3D local_device_detach_cb; - aodev->force =3D 0; - libxl__initiate_device_remove(egc, aodev); - return; - } - /* disk->vdev =3D=3D NULL; fall through */ - default: - /* - * Nothing to do for PHYSTYPE_PHY. - * For other device types assume that the blktap2 process is= - * needed by the soon to be started domain and do nothing. - */ - goto out; + if (disk->vdev !=3D NULL) { + GCNEW(device); + rc =3D libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, + disk, device); + if (rc !=3D 0) goto out; + =20 + aodev->action =3D LIBXL__DEVICE_ACTION_REMOVE; + aodev->dev =3D device; + aodev->callback =3D local_device_detach_cb; + aodev->force =3D 0; + libxl__initiate_device_remove(egc, aodev); + return; } =20 out: --=20 1.9.1 --------------050002040902070707020203 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------050002040902070707020203--