LGTM, thanks On Thu, Oct 8, 2015 at 5:40 AM, 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 | 15 ++++++++++++++- > lib/hypervisor/hv_kvm/monitor.py | 36 > +++++++++++++++++++++++++----------- > 2 files changed, 39 insertions(+), 12 deletions(-) > > diff --git a/lib/hypervisor/hv_kvm/__init__.py > b/lib/hypervisor/hv_kvm/__init__.py > index d81ced4..33cfdc8 100644 > --- a/lib/hypervisor/hv_kvm/__init__.py > +++ b/lib/hypervisor/hv_kvm/__init__.py > @@ -2083,7 +2083,20 @@ 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): > + """Helper function that uses HMP to hot-add a drive.""" > + cmd = "drive_add dummy file=%s,if=none,id=%s,format=raw" % \ > + (filename, kvm_devid) > + self._CallMonitorCommand(instance.name, cmd) > + > + # This must be done indirectly due to the fact that we pass the > drive's > + # file descriptor via QMP first, then we add the corresponding > drive that > + # refers to this fd. Note that if the QMP connection terminates > before > + # a drive which keeps a reference to the fd passed via the add-fd > QMP > + # command has been created, then the fd gets closed and cannot be > used > + # later (e.g., via an drive_add HMP command). > + 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 1d68849..74317f6 100644 > --- a/lib/hypervisor/hv_kvm/monitor.py > +++ b/lib/hypervisor/hv_kvm/monitor.py > @@ -537,12 +537,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): > @@ -556,17 +557,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: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank. This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.
