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.
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
