From: Wei Liu <wei.liu2@citrix.com> To: Anthony PERARD <anthony.perard@citrix.com> Cc: xen-devel@lists.xenproject.org, "Olaf Hering" <olaf@aepfle.de>, "Wei Liu" <wei.liu2@citrix.com>, "Roger Pau Monné" <roger.pau@citrix.com> Subject: Re: [PATCH qemu-xen 4.10 & 4.11] xen_disk: Disable file locking for the PV disk backend Date: Wed, 22 May 2019 15:54:13 +0100 [thread overview] Message-ID: <20190522145413.GA2373@zion.uk.xensource.com> (raw) In-Reply-To: <20190522145140.12943-1-anthony.perard@citrix.com> On Wed, May 22, 2019 at 03:51:40PM +0100, Anthony PERARD wrote: > Since QEMU 2.10 (or qemu-xen-4.10), qemu locks disk images to avoid > them been re-opened in a different qemu process. > > With Xen, there are two issues: > - For HVM guests, a disk image can be open twice! One by the > emulation driver, and one by the PV backend. > - During migration, the qemu process of the newly spawned domain may > attempt to access the disk image before the domain been migrated > and the qemu process are been completely destroyed. > > Migration of HVM guest as been taken care of in libxl, but migration > of PV guest with qdisk and HVM guest attempting to access the PV disk > before unplugging the emulated disk are still an issue. > > For these reasons, we don't want to have QEMU use a locking mechanism > with the PV backend. > > This is already done by db9ff46eeb in QEMU upstream, or QEMU 4.0. > > Affected version of QEMU are: > - qemu-xen of Xen 4.10 and 4.11 > - QEMU 2.10, 2.11, 2.12, 3.0 and 3.1 > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > Hi Olaf, Roger, Wei, > > Instead of a fix in libxl (with the reverted patch "libxl: fix > migration of PV and PVH domUs with and without qemu"), what do you > think of this QEMU patch? It is much simpler that trying to work > around the issue in libxl, and it fix one more problem that still > exist. > > This patch is only for stable branches as Xen unstable already have a > qemu with a similar patch. > > Can you give it a try with one of the affected qemu? (qemu-xen-4.10 or > qemu-xen-4.11) > > Thanks, Thanks for stepping up. :-) > --- > hw/block/xen_disk.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index e431bd89e8..cbff174a98 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -1084,11 +1084,18 @@ static int blk_connect(struct XenDevice *xendev) > Error *local_err = NULL; > QDict *options = NULL; > > + options = qdict_new(); > + > if (strcmp(blkdev->fileproto, "<unset>")) { > - options = qdict_new(); > qdict_put_str(options, "driver", blkdev->fileproto); > } > > + /* > + * It is necessary to turn file locking off as an emulated device > + * may have already opened the same image file. > + */ > + qdict_put_str(options, "file.locking", "off"); > + > /* setup via xenbus -> create new block driver instance */ > xen_pv_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n"); > blkdev->blk = blk_new_open(blkdev->filename, NULL, options, > -- > Anthony PERARD > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: Wei Liu <wei.liu2@citrix.com> To: Anthony PERARD <anthony.perard@citrix.com> Cc: xen-devel@lists.xenproject.org, "Olaf Hering" <olaf@aepfle.de>, "Wei Liu" <wei.liu2@citrix.com>, "Roger Pau Monné" <roger.pau@citrix.com> Subject: Re: [Xen-devel] [PATCH qemu-xen 4.10 & 4.11] xen_disk: Disable file locking for the PV disk backend Date: Wed, 22 May 2019 15:54:13 +0100 [thread overview] Message-ID: <20190522145413.GA2373@zion.uk.xensource.com> (raw) Message-ID: <20190522145413.h2sL7MgyBDQqXMtznk2iBEUthf____c74RZGtaxkQFA@z> (raw) In-Reply-To: <20190522145140.12943-1-anthony.perard@citrix.com> On Wed, May 22, 2019 at 03:51:40PM +0100, Anthony PERARD wrote: > Since QEMU 2.10 (or qemu-xen-4.10), qemu locks disk images to avoid > them been re-opened in a different qemu process. > > With Xen, there are two issues: > - For HVM guests, a disk image can be open twice! One by the > emulation driver, and one by the PV backend. > - During migration, the qemu process of the newly spawned domain may > attempt to access the disk image before the domain been migrated > and the qemu process are been completely destroyed. > > Migration of HVM guest as been taken care of in libxl, but migration > of PV guest with qdisk and HVM guest attempting to access the PV disk > before unplugging the emulated disk are still an issue. > > For these reasons, we don't want to have QEMU use a locking mechanism > with the PV backend. > > This is already done by db9ff46eeb in QEMU upstream, or QEMU 4.0. > > Affected version of QEMU are: > - qemu-xen of Xen 4.10 and 4.11 > - QEMU 2.10, 2.11, 2.12, 3.0 and 3.1 > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > Hi Olaf, Roger, Wei, > > Instead of a fix in libxl (with the reverted patch "libxl: fix > migration of PV and PVH domUs with and without qemu"), what do you > think of this QEMU patch? It is much simpler that trying to work > around the issue in libxl, and it fix one more problem that still > exist. > > This patch is only for stable branches as Xen unstable already have a > qemu with a similar patch. > > Can you give it a try with one of the affected qemu? (qemu-xen-4.10 or > qemu-xen-4.11) > > Thanks, Thanks for stepping up. :-) > --- > hw/block/xen_disk.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index e431bd89e8..cbff174a98 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -1084,11 +1084,18 @@ static int blk_connect(struct XenDevice *xendev) > Error *local_err = NULL; > QDict *options = NULL; > > + options = qdict_new(); > + > if (strcmp(blkdev->fileproto, "<unset>")) { > - options = qdict_new(); > qdict_put_str(options, "driver", blkdev->fileproto); > } > > + /* > + * It is necessary to turn file locking off as an emulated device > + * may have already opened the same image file. > + */ > + qdict_put_str(options, "file.locking", "off"); > + > /* setup via xenbus -> create new block driver instance */ > xen_pv_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n"); > blkdev->blk = blk_new_open(blkdev->filename, NULL, options, > -- > Anthony PERARD > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-05-22 14:54 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-22 14:51 [PATCH qemu-xen 4.10 & 4.11] xen_disk: Disable file locking for the PV disk backend Anthony PERARD 2019-05-22 14:51 ` [Xen-devel] " Anthony PERARD 2019-05-22 14:54 ` Wei Liu [this message] 2019-05-22 14:54 ` Wei Liu 2019-05-22 17:54 ` Olaf Hering 2019-05-22 17:54 ` [Xen-devel] " Olaf Hering 2019-05-23 9:41 ` Anthony PERARD 2019-05-23 9:41 ` [Xen-devel] " Anthony PERARD 2019-06-05 10:53 ` Anthony PERARD 2019-06-14 13:23 ` Olaf Hering
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190522145413.GA2373@zion.uk.xensource.com \ --to=wei.liu2@citrix.com \ --cc=anthony.perard@citrix.com \ --cc=olaf@aepfle.de \ --cc=roger.pau@citrix.com \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).