On Tue, Sep 1, 2015 at 8:09 PM, Dimitris Aragiorgis < [email protected]> wrote:
> * 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. > Agreed. > > > 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 > 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
