Re: [libvirt] [PATCH] qemu: Check for ejected media during startup and migration
On 09/13/2011 10:14 AM, Michal Privoznik wrote:
If the daemon is restarted so we reconnect to monitor, cdrom media
can be ejected. In that case we don't want to show it in domain xml,
or require it on migration destination.
To check for disk status use 'info block' monitor command.
---
NB, the 'info block' is currently not updated yet. The qemu patches
that extend the monitor command are in a queue (that will be merged
quickly):
http://repo.or.cz/w/qemu/kevin.git
head for-anthony
The commit that introduce requested feature:
http://repo.or.cz/w/qemu/kevin.git/commitdiff/e4def80b36231e161b91fa984cd...
Looks like this is in upstream qemu.git now, so that hurdle is out of
the way; guess I'd better give my formal review :)
My original complaint was that this was polling-based, but you've
convinced me that: 1) polling is necessary for libvirtd restart to learn
the correct state (since events while libvirtd is down are lost), and 2)
this doesn't touch user-visible XML, so the polling is limited to a few
points in time, mainly where we were already talking to the monitor anyway.
If, in the future, qemu does add event tracking, we can then expose
user-visible XML to expose this state to the user (whereas this patch
just uses the information internally); at that point, we'd want to only
expose XML if it can avoid polling (since we can't control how
frequently dumpxml is called, and don't want to involve the monitor on
every user-triggered dumpxml). But that's stuff for a later day, which
does not affect this patch.
+++ b/src/qemu/qemu_driver.c
@@ -8236,6 +8236,13 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
goto endjob;
}
+ /* Check if there is and ejected media.
s/and/any/
+int
+qemuDomainCheckEjectableMedia(struct qemud_driver *driver,
+ virDomainObjPtr vm)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret = -1;
+ int i;
+
+ for (i = 0; i< vm->def->ndisks; i++) {
+ virDomainDiskDefPtr disk = vm->def->disks[i];
+ struct qemuDomainDiskInfo info;
+
+ memset(&info, 0, sizeof(info));
+
+ qemuDomainObjEnterMonitor(driver, vm);
This does a monitor command for every disk, even for non-floppy
non-cdrom disks, where we already know the answer (if the disk is not
ejectable, then it can't be in the ejected state). Add a check before
the memset():
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK)
continue;
+ while (*p) {
+ if (STRPREFIX(p, "removable=")) {
+ p += strlen("removable=");
+ if (virStrToLong_i(p,&dummy, 10,&tmp) == -1)
+ VIR_DEBUG("error reading removable: %s", p);
+ else
+ info->removable = p ? true : false;
I tend to write this as "!!p" or "p != NULL", rather than the longer
"p
? true : false", but that's not a show-stopper.
I'm comfortable giving:
ACK with nits fixed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org