[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 1/2] hmp: 'drive_add -n' for creating a node wit
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH 1/2] hmp: 'drive_add -n' for creating a node without BB |
Date: |
Wed, 24 Feb 2016 19:24:46 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 24.02.2016 um 18:50 hat Max Reitz geschrieben:
> On 23.02.2016 18:16, Kevin Wolf wrote:
> > This patch adds an option to the drive_add HMP command to create only a
> > BlockDriverState without a BlockBackend on top.
> >
> > The motivation for this is that libvirt needs to specify options to a
> > migration target (specifically, detect-zeroes). drive-mirror doesn't
> > allow specifying options, and the proper way to do this is to create the
> > target BDS separately with blockdev-add (where you can specify options)
> > and then use blockdev-mirror to that BDS.
> >
> > However, libvirt can't use blockdev-add as long as it is still
> > experimental, and we're expecting that it will still take some time, so
> > we need to resort to drive_add.
> >
> > The problem with drive_add is that so far it always created a BB, and
> > BDSes with a BB can't be used as a mirroring target as long as we don't
> > support multiple BBs per BDS - and while we're working towards that
> > goal, it's another thing that will still take some time.
> >
> > So to achieve the goal, the simplest solution to provide the
> > functionality now without adding one-off options to the mirror QMP
> > commands is to extend drive_add to create nodes without BBs.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > blockdev.c | 30 ++++++++++++++++++++++++++++++
> > device-hotplug.c | 7 +++++++
> > hmp-commands.hx | 4 ++--
> > include/block/block_int.h | 2 ++
> > 4 files changed, 41 insertions(+), 2 deletions(-)
> >
>
> Patch looks good to me (well, except for it being a pity we have to fall
> back on this HMP command), I only have a minor suggestion:
>
> [...]
>
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index bb52e4d..3b44e52 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1200,8 +1200,8 @@ ETEXI
> >
> > {
> > .name = "drive_add",
> > - .args_type = "pci_addr:s,opts:s",
> > - .params = "[[<domain>:]<bus>:]<slot>\n"
> > + .args_type = "node:-n,pci_addr:s,opts:s",
> > + .params = "[-n] [[<domain>:]<bus>:]<slot>\n"
> > "[file=file][,if=type][,bus=n]\n"
> > "[,unit=m][,media=d][,index=i]\n"
> > "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
>
> The description reads:
>
> > Add drive to PCI storage controller.
>
> Maybe this should be extended now?
It was already wrong before this patch, but I guess I could just add
another patch to the series anyway.
Kevin
pgp4mOtK700tj.pgp
Description: PGP signature