* Hrvoje Ribicic <[email protected]> [2015-09-01 12:33:36 +0200]:

> Is there any benefit in passing a function compared to a boolean value?
> What this does is split the special case handling into two places, one of
> which lacks a comment about why it is needed, and the other one with a
> comment but without the actual code.
> 

As you said all I have to do is to move the comment included in the next
patch here.

> On Tue, Aug 18, 2015 at 12:06 PM, Dimitris Aragiorgis <
> [email protected]> wrote:
> 
> > QEMU commits 48f364dd and da2cf4e8 included in version 2.2 and later
> > state the following:
> >
> >  * HMP's drive_del is not supported any more on a drive added
> >    via QMP's blockdev-add
> >
> >  * Stay away from immature blockdev-add unless you want to help
> >    with development.
> >
> > To this end, currently we cannot hot-remove a disk that was
> > previously hot-added. Here, we work around this constrain by using
> > HMP's drive_add instead of blockdev-add. This must be done via a
> > callback in HotAddDisk() wrapper method, due to the fact that if a
> > QMP connection terminates before a drive keeps a reference to the fd
> > passed via the add-fd QMP command, then the fd gets closed and
> > cannot be used later. So we open a QMP connection, pass the fd, then
> > invoke the drive_add HMP command referencing the corresponding
> > fdset, then remove the fdset, then add the device and then terminate
> > the QMP connection.
> >
> > Signed-off-by: Dimitris Aragiorgis <[email protected]>
> > ---
> >  lib/hypervisor/hv_kvm/__init__.py |    8 +++++++-
> >  lib/hypervisor/hv_kvm/monitor.py  |   36
> > +++++++++++++++++++++++++-----------
> >  2 files changed, 32 insertions(+), 12 deletions(-)
> >
> > diff --git a/lib/hypervisor/hv_kvm/__init__.py
> > b/lib/hypervisor/hv_kvm/__init__.py
> > index 4c13253..3bec290 100644
> > --- a/lib/hypervisor/hv_kvm/__init__.py
> > +++ b/lib/hypervisor/hv_kvm/__init__.py
> > @@ -2063,7 +2063,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >      runtime = self._LoadKVMRuntime(instance)
> >      if dev_type == constants.HOTPLUG_TARGET_DISK:
> >        uri = _GetDriveURI(device, extra[0], extra[1])
> > -      self.qmp.HotAddDisk(device, kvm_devid, uri)
> > +
> > +      def drive_add_fn(filename):
> > +        cmd = "drive_add dummy file=%s,if=none,id=%s,format=raw" % \
> > +          (filename, kvm_devid)
> > +        self._CallMonitorCommand(instance.name, cmd)
> > +
> > +      self.qmp.HotAddDisk(device, kvm_devid, uri, drive_add_fn)
> >      elif dev_type == constants.HOTPLUG_TARGET_NIC:
> >        kvmpath = instance.hvparams[constants.HV_KVM_PATH]
> >        kvmhelp = self._GetKVMOutput(kvmpath, self._KVMOPT_HELP)
> > diff --git a/lib/hypervisor/hv_kvm/monitor.py
> > b/lib/hypervisor/hv_kvm/monitor.py
> > index 96dcea8..fd21af5 100644
> > --- a/lib/hypervisor/hv_kvm/monitor.py
> > +++ b/lib/hypervisor/hv_kvm/monitor.py
> > @@ -521,12 +521,13 @@ class QmpConnection(MonitorSocket):
> >      self.Execute("netdev_del", {"id": devid})
> >
> >    @_ensure_connection
> > -  def HotAddDisk(self, disk, devid, uri):
> > +  def HotAddDisk(self, disk, devid, uri, drive_add_fn=None):
> >      """Hot-add a disk
> >
> >      Try opening the device to obtain a fd and pass it with SCM_RIGHTS.
> > This
> >      will be omitted in case of userspace access mode (open will fail).
> > -    Then use blockdev-add and then device_add.
> > +    Then use blockdev-add QMP command or drive_add_fn() callback if any.
> > +    The add the guest device.
> >
> >      """
> >      if os.path.exists(uri):
> > @@ -540,17 +541,30 @@ class QmpConnection(MonitorSocket):
> >        filename = uri
> >        fdset = None
> >
> > -    arguments = {
> > -      "options": {
> > -        "driver": "raw",
> > -        "id": devid,
> > -        "file": {
> > -          "driver": "file",
> > -          "filename": filename,
> > +    # FIXME: Use blockdev-add/blockdev-del when properly implemented in
> > QEMU.
> > +    # This is an ugly hack to work around QEMU commits 48f364dd and
> > da2cf4e8:
> > +    #  * HMP's drive_del is not supported any more on a drive added
> > +    #    via QMP's blockdev-add
> > +    #  * Stay away from immature blockdev-add unless you want to help
> > +    #     with development.
> > +    # Using drive_add here must be done via a callback due to the fact
> > that if
> > +    # a QMP connection terminates before a drive keeps a reference to the
> > fd
> > +    # passed via the add-fd QMP command, then the fd gets closed and
> > +    # cannot be used later.
> > +    if drive_add_fn:
> > +      drive_add_fn(filename)
> > +    else:
> > +      arguments = {
> > +        "options": {
> > +          "driver": "raw",
> > +          "id": devid,
> > +          "file": {
> > +            "driver": "file",
> > +            "filename": filename,
> > +          }
> >          }
> >        }
> > -    }
> > -    self.Execute("blockdev-add", arguments)
> > +      self.Execute("blockdev-add", arguments)
> >
> >      if fdset is not None:
> >        self._RemoveFdset(fdset)
> > --
> > 1.7.10.4
> >
> >
> Hrvoje Ribicic
> Ganeti Engineering
> Google Germany GmbH
> Dienerstr. 12, 80331, München
> 
> Geschäftsführer: Graham Law, Christine Elizabeth Flores
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg

Attachment: signature.asc
Description: Digital signature

Reply via email to