[libvirt] [PATCH] qemu: Check for ejected media during startup and migration
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/e4def80b36231e161b91fa984cd0d7... src/qemu/qemu_conf.h | 6 +++ src/qemu/qemu_driver.c | 7 +++ src/qemu/qemu_hotplug.c | 31 ++++++++++++++ src/qemu/qemu_hotplug.h | 2 + src/qemu/qemu_monitor.c | 19 +++++++++ src/qemu/qemu_monitor.h | 4 ++ src/qemu/qemu_monitor_json.c | 89 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_monitor_text.c | 90 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 + src/qemu/qemu_process.c | 3 + 11 files changed, 257 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index e8b92a4..ff5cf23 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -165,4 +165,10 @@ void qemuDriverUnlock(struct qemud_driver *driver); int qemudLoadDriverConfig(struct qemud_driver *driver, const char *filename); +struct qemuDomainDiskInfo { + bool removable; + bool locked; + bool tray_open; +}; + #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b94d1c4..b10a56f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8236,6 +8236,13 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto endjob; } + /* Check if there is and ejected media. + * We don't want to require them on the destination. + */ + + if (qemuDomainCheckEjectableMedia(driver, vm) < 0) + goto endjob; + if (!(xml = qemuMigrationBegin(driver, vm, xmlin, cookieout, cookieoutlen))) goto endjob; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6cfe392..91ac78a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -150,6 +150,37 @@ error: return -1; } +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); + if (qemuMonitorGetBlockInfo(priv->mon, disk->info.alias, &info) < 0) { + qemuDomainObjExitMonitor(driver, vm); + goto cleanup; + } + qemuDomainObjExitMonitor(driver, vm); + + if (info.tray_open && disk->src) + VIR_FREE(disk->src); + } + + ret = 0; + +cleanup: + return ret; +} + int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 65d1d30..aaaed88 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -31,6 +31,8 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, bool force); +int qemuDomainCheckEjectableMedia(struct qemud_driver *driver, + virDomainObjPtr vm); int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fca8fa4..7005564 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1215,6 +1215,25 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return ret; } +int qemuMonitorGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info) +{ + int ret; + + VIR_DEBUG("mon=%p dev=%p info=%p", mon, devname, info); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetBlockInfo(mon, devname, info); + else + ret = qemuMonitorTextGetBlockInfo(mon, devname, info); + return ret; +} int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 390eeff..a05d548 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -28,6 +28,7 @@ # include "internal.h" # include "domain_conf.h" +# include "qemu_conf.h" # include "hash.h" typedef struct _qemuMonitor qemuMonitor; @@ -212,6 +213,9 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info); int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a81ec8..812a78e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1337,6 +1337,95 @@ cleanup: } +int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info) +{ + int ret = 0; + bool found = false; + int i; + + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-block", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + if (ret < 0) + goto cleanup; + + ret = -1; + + devices = virJSONValueObjectGet(reply, "return"); + if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block info reply was missing device list")); + goto cleanup; + } + + for (i = 0; i < virJSONValueArraySize(devices); i++) { + virJSONValuePtr dev = virJSONValueArrayGet(devices, i); + const char *thisdev; + + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block info device entry was not in expected format")); + goto cleanup; + } + + if ((thisdev = virJSONValueObjectGetString(dev, "device")) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block info device entry was not in expected format")); + goto cleanup; + } + + if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) + thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STRNEQ(thisdev, devname)) + continue; + + found = true; + if (virJSONValueObjectGetBoolean(dev, "removable", &info->removable) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s value"), + "removable"); + goto cleanup; + } + + if (virJSONValueObjectGetBoolean(dev, "locked", &info->locked) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s value"), + "locked"); + goto cleanup; + } + + /* Don't check for success here, because 'tray-open' is presented iff + * medium is ejected. + */ + virJSONValueObjectGetBoolean(dev, "tray-open", &info->tray_open); + + break; + } + + if (!found) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find info for device '%s'"), + devname); + goto cleanup; + } + + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index dfeba7e..24d6aec 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -60,6 +60,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info); int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 0b50ba2..50b9836 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -750,6 +750,96 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, } +int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info) +{ + char *reply = NULL; + int ret = -1; + char *dummy; + const char *p, *eol; + int devnamelen = strlen(devname); + int tmp; + + if (qemuMonitorHMPCommand(mon, "info block", &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("info block command failed")); + goto cleanup; + } + + if (strstr(reply, "\ninfo ")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", + _("info block not supported by this qemu")); + goto cleanup; + } + + /* The output looks like this: + * drive-ide0-0-0: removable=0 file=<path> ro=0 drv=raw encrypted=0 + * drive-ide0-1-0: removable=1 locked=0 file=<path> ro=1 drv=raw encrypted=0 + */ + p = reply; + + while (*p) { + if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) + p += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STREQLEN(p, devname, devnamelen) && + p[devnamelen] == ':' && p[devnamelen+1] == ' ') { + + eol = strchr(p, '\n'); + if (!eol) + eol = p + strlen(p); + + p += devnamelen + 2; /*Skip to first label. */ + + 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; + } else if (STRPREFIX(p, "locked=")) { + p += strlen("locked="); + if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) + VIR_DEBUG("error reading locked: %s", p); + else + info->locked = p ? true : false; + } else if (STRPREFIX(p, "tray_open=")) { + p += strlen("tray_open="); + if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) + VIR_DEBUG("error reading tray_open: %s", p); + else + info->tray_open = p ? true : false; + } else { + /* ignore because we don't parse all options */ + } + + /* skip to next label */ + p = strchr(p, ' '); + if (!p || p >= eol) break; + p++; + } + + ret = 0; + goto cleanup; + } + + /* skip to next line */ + p = strchr(p, '\n'); + if (!p) break; + p++; + } + + qemuReportError(VIR_ERR_INVALID_ARG, + _("no info for device '%s'"), devname); + +cleanup: + VIR_FREE(reply); + return ret; +} + int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 7cc3172..68e16b3 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -57,6 +57,9 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info); int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8a8475..7b5d10a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2612,6 +2612,9 @@ qemuProcessReconnect(void *opaque) if (qemuProcessFiltersInstantiate(conn, obj->def)) goto error; + if (qemuDomainCheckEjectableMedia(driver, obj) < 0) + goto error; + if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0) goto error; -- 1.7.3.4
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.
Yuck - this is still polling-based (we have to ask qemu every time we want to dumpxml). Whatever happened to the proposal of having interrupt-based cdrom events, where qemu sends an event any time the virtual tray changes state (whether by monitor command or guest-initiated), and libvirt monitors those events to update its internal state at that time, so that libvirt's internal state is always accurate? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 09/13/2011 05:03 PM, Eric Blake wrote:
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.
Yuck - this is still polling-based (we have to ask qemu every time we want to dumpxml). Whatever happened to the proposal of having interrupt-based cdrom events, where qemu sends an event any time the virtual tray changes state (whether by monitor command or guest-initiated), and libvirt monitors those events to update its internal state at that time, so that libvirt's internal state is always accurate?
For reference: https://www.redhat.com/archives/libvir-list/2011-August/msg00487.html -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 14.09.2011 01:05, Eric Blake wrote:
On 09/13/2011 05:03 PM, Eric Blake wrote:
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.
Yuck - this is still polling-based (we have to ask qemu every time we want to dumpxml). Whatever happened to the proposal of having interrupt-based cdrom events, where qemu sends an event any time the virtual tray changes state (whether by monitor command or guest-initiated), and libvirt monitors those events to update its internal state at that time, so that libvirt's internal state is always accurate?
For reference: https://www.redhat.com/archives/libvir-list/2011-August/msg00487.html
My patch does not touch dumpxml. It access monitor only on process reconnect and in early migration phase. In both cases we access monitor anyway. But I agree that events would be nicer, but I don't think they are gonna be implemented in near future. However, even if they would have been implemented now, we need to check the tray during process reconnect. Because if the libvirtd was down and during this time guest ejected cdrom and qemu sent event, we would not catch it - because we are not running. So in order to keep things consistent, we must check tray during startup. Migration is the same. Even if qemu will sent events at one time, we still need to check if we are talking to older qemu. Michal
On 09/14/2011 01:52 AM, Michal Privoznik wrote:
On 14.09.2011 01:05, Eric Blake wrote:
On 09/13/2011 05:03 PM, Eric Blake wrote:
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.
Yuck - this is still polling-based (we have to ask qemu every time we want to dumpxml). Whatever happened to the proposal of having interrupt-based cdrom events, where qemu sends an event any time the virtual tray changes state (whether by monitor command or guest-initiated), and libvirt monitors those events to update its internal state at that time, so that libvirt's internal state is always accurate?
For reference: https://www.redhat.com/archives/libvir-list/2011-August/msg00487.html
My patch does not touch dumpxml. It access monitor only on process reconnect and in early migration phase. In both cases we access monitor anyway.
That is, you are polling the status at any location where you would be doing an internal dumpxml. So while this is not exposing things to the user (and thus less polling), it is still doing polling. I'm also worried about virDomainSave and virDomainSnapshotCreateXML needing to be hooked into doing these sorts of polling, to work correctly. I'd feel much better revisiting this issue post-0.9.5 - it's too invasive to fix right now.
But I agree that events would be nicer, but I don't think they are gonna be implemented in near future. However, even if they would have been implemented now, we need to check the tray during process reconnect.
You have a point there - but that is a one-time poll, rather than on every operation that needs to preserve xml state.
Because if the libvirtd was down and during this time guest ejected cdrom and qemu sent event, we would not catch it - because we are not running. So in order to keep things consistent, we must check tray during startup. Migration is the same. Even if qemu will sent events at one time, we still need to check if we are talking to older qemu.
This should be capability-based - we should know whether the qemu we are talking to supports nothing (0.14), polling (is it in qemu yet? or still just pending), or events (even further down the road), and make appropriate decisions on how much to probe according to the qemu we are talking to. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 16.09.2011 17:38, Eric Blake wrote:
On 09/14/2011 01:52 AM, Michal Privoznik wrote:
On 14.09.2011 01:05, Eric Blake wrote:
On 09/13/2011 05:03 PM, Eric Blake wrote:
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.
Yuck - this is still polling-based (we have to ask qemu every time we want to dumpxml). Whatever happened to the proposal of having interrupt-based cdrom events, where qemu sends an event any time the virtual tray changes state (whether by monitor command or guest-initiated), and libvirt monitors those events to update its internal state at that time, so that libvirt's internal state is always accurate?
For reference: https://www.redhat.com/archives/libvir-list/2011-August/msg00487.html
My patch does not touch dumpxml. It access monitor only on process reconnect and in early migration phase. In both cases we access monitor anyway.
That is, you are polling the status at any location where you would be doing an internal dumpxml. So while this is not exposing things to the user (and thus less polling), it is still doing polling. I'm also worried about virDomainSave and virDomainSnapshotCreateXML needing to be hooked into doing these sorts of polling, to work correctly.
I'd feel much better revisiting this issue post-0.9.5 - it's too invasive to fix right now.
But I agree that events would be nicer, but I don't think they are gonna be implemented in near future. However, even if they would have been implemented now, we need to check the tray during process reconnect.
You have a point there - but that is a one-time poll, rather than on every operation that needs to preserve xml state.
Because if the libvirtd was down and during this time guest ejected cdrom and qemu sent event, we would not catch it - because we are not running. So in order to keep things consistent, we must check tray during startup. Migration is the same. Even if qemu will sent events at one time, we still need to check if we are talking to older qemu.
This should be capability-based - we should know whether the qemu we are talking to supports nothing (0.14), polling (is it in qemu yet? or still just pending), or events (even further down the road), and make appropriate decisions on how much to probe according to the qemu we are talking to.
So I think the conclusion is: we need both, extended 'info block' (to do one-time poll on startup) and events (any later on). And I agree with that. However, qemu implements only the first part - extended 'info block' command so far. As soon as it will implement events too, we can adapt to them => make the check in migration early phase dependent on qemu capabilities. But I think the current patch is the best we can do right now. Michal
On 13.09.2011 18:14, 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/e4def80b36231e161b91fa984cd0d7...
src/qemu/qemu_conf.h | 6 +++ src/qemu/qemu_driver.c | 7 +++ src/qemu/qemu_hotplug.c | 31 ++++++++++++++ src/qemu/qemu_hotplug.h | 2 + src/qemu/qemu_monitor.c | 19 +++++++++ src/qemu/qemu_monitor.h | 4 ++ src/qemu/qemu_monitor_json.c | 89 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_monitor_text.c | 90 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 + src/qemu/qemu_process.c | 3 + 11 files changed, 257 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index e8b92a4..ff5cf23 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -165,4 +165,10 @@ void qemuDriverUnlock(struct qemud_driver *driver); int qemudLoadDriverConfig(struct qemud_driver *driver, const char *filename);
+struct qemuDomainDiskInfo { + bool removable; + bool locked; + bool tray_open; +}; + #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b94d1c4..b10a56f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8236,6 +8236,13 @@ qemuDomainMigrateBegin3(virDomainPtr domain, goto endjob; }
+ /* Check if there is and ejected media. + * We don't want to require them on the destination. + */ + + if (qemuDomainCheckEjectableMedia(driver, vm) < 0) + goto endjob; + if (!(xml = qemuMigrationBegin(driver, vm, xmlin, cookieout, cookieoutlen))) goto endjob; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6cfe392..91ac78a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -150,6 +150,37 @@ error: return -1; }
+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); + if (qemuMonitorGetBlockInfo(priv->mon, disk->info.alias, &info) < 0) { + qemuDomainObjExitMonitor(driver, vm); + goto cleanup; + } + qemuDomainObjExitMonitor(driver, vm); + + if (info.tray_open && disk->src) + VIR_FREE(disk->src); + } + + ret = 0; + +cleanup: + return ret; +} +
int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 65d1d30..aaaed88 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -31,6 +31,8 @@ int qemuDomainChangeEjectableMedia(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, bool force); +int qemuDomainCheckEjectableMedia(struct qemud_driver *driver, + virDomainObjPtr vm); int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index fca8fa4..7005564 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1215,6 +1215,25 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return ret; }
+int qemuMonitorGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info) +{ + int ret; + + VIR_DEBUG("mon=%p dev=%p info=%p", mon, devname, info); + if (!mon) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("monitor must not be NULL")); + return -1; + } + + if (mon->json) + ret = qemuMonitorJSONGetBlockInfo(mon, devname, info); + else + ret = qemuMonitorTextGetBlockInfo(mon, devname, info); + return ret; +}
int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 390eeff..a05d548 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -28,6 +28,7 @@ # include "internal.h"
# include "domain_conf.h" +# include "qemu_conf.h" # include "hash.h"
typedef struct _qemuMonitor qemuMonitor; @@ -212,6 +213,9 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info); int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a81ec8..812a78e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1337,6 +1337,95 @@ cleanup: }
+int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info) +{ + int ret = 0; + bool found = false; + int i; + + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-block", + NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr devices; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + if (ret < 0) + goto cleanup; + + ret = -1; + + devices = virJSONValueObjectGet(reply, "return"); + if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block info reply was missing device list")); + goto cleanup; + } + + for (i = 0; i < virJSONValueArraySize(devices); i++) { + virJSONValuePtr dev = virJSONValueArrayGet(devices, i); + const char *thisdev; + + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block info device entry was not in expected format")); + goto cleanup; + } + + if ((thisdev = virJSONValueObjectGetString(dev, "device")) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("block info device entry was not in expected format")); + goto cleanup; + } + + if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) + thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STRNEQ(thisdev, devname)) + continue; + + found = true; + if (virJSONValueObjectGetBoolean(dev, "removable", &info->removable) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s value"), + "removable"); + goto cleanup; + } + + if (virJSONValueObjectGetBoolean(dev, "locked", &info->locked) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s value"), + "locked"); + goto cleanup; + } + + /* Don't check for success here, because 'tray-open' is presented iff + * medium is ejected. + */ + virJSONValueObjectGetBoolean(dev, "tray-open", &info->tray_open); + + break; + } + + if (!found) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find info for device '%s'"), + devname); + goto cleanup; + } + + ret = 0; + +cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index dfeba7e..24d6aec 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -60,6 +60,9 @@ int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info); int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 0b50ba2..50b9836 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -750,6 +750,96 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, }
+int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info) +{ + char *reply = NULL; + int ret = -1; + char *dummy; + const char *p, *eol; + int devnamelen = strlen(devname); + int tmp; + + if (qemuMonitorHMPCommand(mon, "info block", &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + "%s", _("info block command failed")); + goto cleanup; + } + + if (strstr(reply, "\ninfo ")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", + _("info block not supported by this qemu")); + goto cleanup; + } + + /* The output looks like this: + * drive-ide0-0-0: removable=0 file=<path> ro=0 drv=raw encrypted=0 + * drive-ide0-1-0: removable=1 locked=0 file=<path> ro=1 drv=raw encrypted=0 + */ + p = reply; + + while (*p) { + if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) + p += strlen(QEMU_DRIVE_HOST_PREFIX); + + if (STREQLEN(p, devname, devnamelen) && + p[devnamelen] == ':' && p[devnamelen+1] == ' ') { + + eol = strchr(p, '\n'); + if (!eol) + eol = p + strlen(p); + + p += devnamelen + 2; /*Skip to first label. */ + + 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; + } else if (STRPREFIX(p, "locked=")) { + p += strlen("locked="); + if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) + VIR_DEBUG("error reading locked: %s", p); + else + info->locked = p ? true : false; + } else if (STRPREFIX(p, "tray_open=")) { + p += strlen("tray_open="); + if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) + VIR_DEBUG("error reading tray_open: %s", p); + else + info->tray_open = p ? true : false; + } else { + /* ignore because we don't parse all options */ + } + + /* skip to next label */ + p = strchr(p, ' '); + if (!p || p >= eol) break; + p++; + } + + ret = 0; + goto cleanup; + } + + /* skip to next line */ + p = strchr(p, '\n'); + if (!p) break; + p++; + } + + qemuReportError(VIR_ERR_INVALID_ARG, + _("no info for device '%s'"), devname); + +cleanup: + VIR_FREE(reply); + return ret; +} + int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 7cc3172..68e16b3 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -57,6 +57,9 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, + const char *devname, + struct qemuDomainDiskInfo *info); int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *devname, long long *rd_req, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f8a8475..7b5d10a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2612,6 +2612,9 @@ qemuProcessReconnect(void *opaque) if (qemuProcessFiltersInstantiate(conn, obj->def)) goto error;
+ if (qemuDomainCheckEjectableMedia(driver, obj) < 0) + goto error; + if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0) goto error;
Ping?
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/e4def80b36231e161b91fa984cd0d7...
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik