[libvirt] [PATCH v2 0/2] Fix migration to older libvirt
Recently I've implemented new feature that we can set "vgamem_mb" for QXL device and also I've fixed libvirt to honor the 'vram' attribute and pass that value to QEMU process. But with this change the migration to older libvirt stopped working because QEMU silently updates the video memory size if the value is too low or there are some dependencies (for example QXL device has to have 'ram' size twice as 'vgamem'). To fix the migration we need to load the updated values from QEMU and store them into the status XML. v1: - removed unnecessary movement of qemuMonitorJSONObjectProperty - completely rewritten the second patch Pavel Hrdina (2): qemu_monitor: introduce new function to get QOM path qemu_process: detect updated video ram size values from QEMU src/qemu/qemu_monitor.c | 210 ++++++++++++++++++++++++++++--------------- src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_monitor_json.c | 69 ++++++++++++++ src/qemu/qemu_monitor_json.h | 3 + src/qemu/qemu_process.c | 71 +++++++++++++++ 5 files changed, 287 insertions(+), 70 deletions(-) -- 2.0.5
The search is done recursively only through QOM object that has a type prefixed with "child<" as this indicate that the QOM is a parent for other QOM objects. The usage is that you give known device name with starting path where to search. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor.c | 172 ++++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 70 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 57a13ab..55f07f3 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1008,94 +1008,52 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options) mon->options = options; } -/* Search the qom objects for the balloon driver object by it's known name - * of "virtio-balloon-pci". The entry for the driver will be found in the - * returned 'type' field using the syntax "child<virtio-balloon-pci>". - * - * Once found, check the entry to ensure it has the correct property listed. - * If it does not, then obtaining statistics from qemu will not be possible. - * This feature was added to qemu 1.5. + +/** + * Search the qom objects by it's known name. The name is compared against + * filed 'type' formatted as 'link<%name>'. * * This procedure will be call recursively until found or the qom-list is * exhausted. * * Returns: * - * 1 - Found - * 0 - Not found still looking + * 0 - Found * -1 - Error bail out + * -2 - Not found * * NOTE: This assumes we have already called qemuDomainObjEnterMonitor() */ static int -qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, - virDomainObjPtr vm, - const char *curpath) +qemuMonitorFindObjectPath(qemuMonitorPtr mon, + const char *curpath, + const char *name, + char **path) { - ssize_t i, j, npaths = 0, nprops = 0; - int ret = 0; + ssize_t i, npaths = 0; + int ret = -2; char *nextpath = NULL; + char *type = NULL; qemuMonitorJSONListPathPtr *paths = NULL; - qemuMonitorJSONListPathPtr *bprops = NULL; - - if (mon->balloonpath) { - return 1; - } else if (mon->ballooninit) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot determine balloon device path")); - return -1; - } - /* Not supported */ - if (!vm->def->memballoon || - vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Memory balloon model must be virtio to " - "get memballoon path")); + if (virAsprintf(&type, "link<%s>", name) < 0) return -1; - } - VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath); + VIR_DEBUG("Searching for '%s' Object Path starting at '%s'", type, curpath); npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths); if (npaths < 0) - return -1; + goto cleanup; - for (i = 0; i < npaths && ret == 0; i++) { + for (i = 0; i < npaths && ret == -2; i++) { - if (STREQ_NULLABLE(paths[i]->type, "link<virtio-balloon-pci>")) { - VIR_DEBUG("Path to <virtio-balloon-pci> is '%s/%s'", - curpath, paths[i]->name); - if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name) < 0) { + if (STREQ_NULLABLE(paths[i]->type, type)) { + VIR_DEBUG("Path to '%s' is '%s/%s'", type, curpath, paths[i]->name); + ret = 0; + if (virAsprintf(path, "%s/%s", curpath, paths[i]->name) < 0) { + *path = NULL; ret = -1; - goto cleanup; - } - - /* Now look at the each of the property entries to determine - * whether "guest-stats-polling-interval" exists. If not, - * then this version of qemu/kvm does not support the feature. - */ - nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath, &bprops); - if (nprops < 0) { - ret = -1; - goto cleanup; - } - - for (j = 0; j < nprops; j++) { - if (STREQ(bprops[j]->name, "guest-stats-polling-interval")) { - VIR_DEBUG("Found Balloon Object Path %s", nextpath); - mon->balloonpath = nextpath; - nextpath = NULL; - ret = 1; - goto cleanup; - } } - - /* If we get here, we found the path, but not the property */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Property 'guest-stats-polling-interval' " - "not found on memory balloon driver.")); - ret = -1; goto cleanup; } @@ -1107,7 +1065,8 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, ret = -1; goto cleanup; } - ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath); + + ret = qemuMonitorFindObjectPath(mon, nextpath, name, path); } } @@ -1115,10 +1074,83 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, for (i = 0; i < npaths; i++) qemuMonitorJSONListPathFree(paths[i]); VIR_FREE(paths); - for (j = 0; j < nprops; j++) - qemuMonitorJSONListPathFree(bprops[j]); - VIR_FREE(bprops); VIR_FREE(nextpath); + VIR_FREE(type); + return ret; +} + + +/** + * Search the qom objects for the balloon driver object by it's known name + * of "virtio-balloon-pci". The entry for the driver will be found by using + * function "qemuMonitorFindObjectPath". + * + * Once found, check the entry to ensure it has the correct property listed. + * If it does not, then obtaining statistics from QEMU will not be possible. + * This feature was added to QEMU 1.5. + * + * Returns: + * + * 0 - Found + * -1 - Not found or error + * + * NOTE: This assumes we have already called qemuDomainObjEnterMonitor() + */ +static int +qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, + const char *curpath) +{ + ssize_t i, nprops = 0; + int ret = -1; + char *path = NULL; + qemuMonitorJSONListPathPtr *bprops = NULL; + virDomainObjPtr vm = mon->vm; + + if (mon->balloonpath) { + return 0; + } else if (mon->ballooninit) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot determine balloon device path")); + return -1; + } + + /* Not supported */ + if (!vm->def->memballoon || + vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Memory balloon model must be virtio to " + "get memballoon path")); + return -1; + } + + if (qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-pci", &path) < 0) + return -1; + + nprops = qemuMonitorJSONGetObjectListPaths(mon, path, &bprops); + if (nprops < 0) + goto cleanup; + + for (i = 0; i < nprops; i++) { + if (STREQ(bprops[i]->name, "guest-stats-polling-interval")) { + VIR_DEBUG("Found Balloon Object Path %s", path); + mon->balloonpath = path; + path = NULL; + ret = 0; + goto cleanup; + } + } + + + /* If we get here, we found the path, but not the property */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Property 'guest-stats-polling-interval' " + "not found on memory balloon driver.")); + + cleanup: + for (i = 0; i < nprops; i++) + qemuMonitorJSONListPathFree(bprops[i]); + VIR_FREE(bprops); + VIR_FREE(path); return ret; } @@ -1632,7 +1664,7 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, } if (mon->json) { - ignore_value(qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/")); + ignore_value(qemuMonitorFindBalloonObjectPath(mon, "/")); mon->ballooninit = true; ret = qemuMonitorJSONGetMemoryStats(mon, mon->balloonpath, stats, nr_stats); @@ -1660,7 +1692,7 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, return -1; } - if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) { + if (qemuMonitorFindBalloonObjectPath(mon, "/") == 0) { ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath, period); } -- 2.0.5
QEMU internally updates the size of video memory if the domain XML had provided too low memory size or there are some dependencies for a QXL devices 'vgamem' and 'ram' size. We need to know about the changes and store them into the status XML to not break migration or managedsave through different libvirt versions. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor.c | 38 ++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_process.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55f07f3..45bb62f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1154,6 +1154,44 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, return ret; } + +/** + * To update video memory size in status XML we need to load correct values from + * QEMU. This is supported only with JSON monitor. + * + * Returns 0 on success, -1 on failure and sets proper error message. + */ +int +qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + const char *videoName) +{ + int ret = -1; + char *path = NULL; + + if (mon->json) { + if (qemuMonitorFindObjectPath(mon, "/", videoName, &path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find QOM Object path for device '%s'"), + videoName); + return -1; + } + if (qemuMonitorJSONUpdateVideoMemorySize(mon, video, path) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + } + + ret = 0; + + cleanup: + VIR_FREE(path); + return ret; +} + + int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, const char *cmd, int scm_fd, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index edab66f..133d42d 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -243,6 +243,10 @@ virJSONValuePtr qemuMonitorGetOptions(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); void qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options) ATTRIBUTE_NONNULL(1); +int qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + const char *videName) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, const char *cmd, int scm_fd, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e567aa7..da5c14d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1300,6 +1300,75 @@ int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, } +/** + * Loads correct video memory size values from QEMU and update the video + * definition. + * + * Return 0 on success, -1 on failure and set proper error message. + */ +int +qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + char *path) +{ + qemuMonitorJSONObjectProperty prop = { + QEMU_MONITOR_OBJECT_PROPERTY_ULONG, + {0} + }; + + switch (video->type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + if (qemuMonitorJSONGetObjectProperty(mon, path, "vgamem_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Objext '%s' has no property 'vgamem_mb'"), + path); + return -1; + } + video->vram = prop.val.ul * 1024; + break; + case VIR_DOMAIN_VIDEO_TYPE_QXL: + if (qemuMonitorJSONGetObjectProperty(mon, path, "vram_size", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Objext '%s' has no property 'vram_size'"), + path); + return -1; + } + video->vram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, "ram_size", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Objext '%s' has no property 'ram_size'"), + path); + return -1; + } + video->ram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, "vgamem_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Objext '%s' has no property 'vgamem_mb'"), + path); + return -1; + } + video->vgamem = prop.val.ul * 1024; + break; + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + if (qemuMonitorJSONGetObjectProperty(mon, path, "vgamem_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Objext '%s' has no property 'vgamem_mb'"), + path); + return -1; + } + video->vram = prop.val.ul * 1024; + break; + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_XEN: + case VIR_DOMAIN_VIDEO_TYPE_VBOX: + case VIR_DOMAIN_VIDEO_TYPE_LAST: + break; + } + + return 0; +} + + /* * Returns: 0 if balloon not supported, +1 if balloon query worked * or -1 on failure diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 222f11e..1da1a00 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -57,6 +57,9 @@ int qemuMonitorJSONGetCPUInfo(qemuMonitorPtr mon, int **pids); int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon, int *virtType); +int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + char *path); int qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon, unsigned long long *currmem); int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c18204b..65c907d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3067,6 +3067,73 @@ qemuProcessCleanupChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, } +/** + * Loads and update video memory size for video devices according to QEMU + * process as the QEMU will silently update the values that we pass to QEMU + * through command line. We need to load these updated values and store them + * into the status XML. + * + * We will fail if for some reason the values cannot be loaded from QEMU because + * its mandatory to get the correct video memory size to status XML to not break + * migration. + */ +static int +qemuProcessUpdateVideoRamSize(virQEMUDriverPtr driver, + virDomainObjPtr vm, + int asyncJob) +{ + ssize_t i; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainVideoDefPtr video = NULL; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *path = NULL; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + for (i = 0; i < vm->def->nvideos; i++) { + video = vm->def->videos[i]; + + switch (video->type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + if (qemuMonitorUpdateVideoMemorySize(priv->mon, video, "VGA") < 0) + goto error; + break; + case VIR_DOMAIN_VIDEO_TYPE_QXL: + if (qemuMonitorUpdateVideoMemorySize(priv->mon, video, + video->primary ? "qxl-vga" : "qxl") < 0) + goto error; + break; + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + if (qemuMonitorUpdateVideoMemorySize(priv->mon, video, + "vmware-svga") < 0) + goto error; + break; + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_XEN: + case VIR_DOMAIN_VIDEO_TYPE_VBOX: + case VIR_DOMAIN_VIDEO_TYPE_LAST: + break; + } + + VIR_FREE(path); + path = NULL; + } + + qemuDomainObjExitMonitor(driver, vm); + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + return -1; + + return 0; + + error: + qemuDomainObjExitMonitor(driver, vm); + VIR_FREE(path); + return -1; +} + + struct qemuProcessHookData { virConnectPtr conn; virDomainObjPtr vm; @@ -4801,6 +4868,10 @@ int qemuProcessStart(virConnectPtr conn, } qemuDomainObjExitMonitor(driver, vm); + VIR_DEBUG("Detecting actual memory size for video device"); + if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) + goto cleanup; + if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) { VIR_DEBUG("Starting domain CPUs"); /* Allow the CPUS to start executing */ -- 2.0.5
On Fri, Jan 09, 2015 at 10:02:07AM +0100, Pavel Hrdina wrote:
QEMU internally updates the size of video memory if the domain XML had provided too low memory size or there are some dependencies for a QXL devices 'vgamem' and 'ram' size. We need to know about the changes and store them into the status XML to not break migration or managedsave through different libvirt versions.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor.c | 38 ++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_process.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55f07f3..45bb62f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1154,6 +1154,44 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, return ret; }
+ +/** + * To update video memory size in status XML we need to load correct values from + * QEMU. This is supported only with JSON monitor. + * + * Returns 0 on success, -1 on failure and sets proper error message. + */ +int +qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + const char *videoName) +{ + int ret = -1; + char *path = NULL; + + if (mon->json) { + if (qemuMonitorFindObjectPath(mon, "/", videoName, &path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find QOM Object path for device '%s'"), + videoName); + return -1; + } + if (qemuMonitorJSONUpdateVideoMemorySize(mon, video, path) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + }
This fatal error is going to break all guest startup with non-json monitor IIUC. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/09/2015 10:51 AM, Daniel P. Berrange wrote:
On Fri, Jan 09, 2015 at 10:02:07AM +0100, Pavel Hrdina wrote:
QEMU internally updates the size of video memory if the domain XML had provided too low memory size or there are some dependencies for a QXL devices 'vgamem' and 'ram' size. We need to know about the changes and store them into the status XML to not break migration or managedsave through different libvirt versions.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor.c | 38 ++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_process.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55f07f3..45bb62f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1154,6 +1154,44 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, return ret; }
+ +/** + * To update video memory size in status XML we need to load correct values from + * QEMU. This is supported only with JSON monitor. + * + * Returns 0 on success, -1 on failure and sets proper error message. + */ +int +qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + const char *videoName) +{ + int ret = -1; + char *path = NULL; + + if (mon->json) { + if (qemuMonitorFindObjectPath(mon, "/", videoName, &path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find QOM Object path for device '%s'"), + videoName); + return -1; + } + if (qemuMonitorJSONUpdateVideoMemorySize(mon, video, path) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + }
This fatal error is going to break all guest startup with non-json monitor IIUC.
Regards, Daniel
Yes I know that. Should we return 0 and still reporting that error or should we just skip it at all if we cannot use mon-json? Pavel
On Fri, Jan 09, 2015 at 02:08:06PM +0100, Pavel Hrdina wrote:
On 01/09/2015 10:51 AM, Daniel P. Berrange wrote:
On Fri, Jan 09, 2015 at 10:02:07AM +0100, Pavel Hrdina wrote:
QEMU internally updates the size of video memory if the domain XML had provided too low memory size or there are some dependencies for a QXL devices 'vgamem' and 'ram' size. We need to know about the changes and store them into the status XML to not break migration or managedsave through different libvirt versions.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor.c | 38 ++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_process.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55f07f3..45bb62f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1154,6 +1154,44 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, return ret; }
+ +/** + * To update video memory size in status XML we need to load correct values from + * QEMU. This is supported only with JSON monitor. + * + * Returns 0 on success, -1 on failure and sets proper error message. + */ +int +qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + const char *videoName) +{ + int ret = -1; + char *path = NULL; + + if (mon->json) { + if (qemuMonitorFindObjectPath(mon, "/", videoName, &path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find QOM Object path for device '%s'"), + videoName); + return -1; + } + if (qemuMonitorJSONUpdateVideoMemorySize(mon, video, path) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + }
This fatal error is going to break all guest startup with non-json monitor IIUC.
Regards, Daniel
Yes I know that. Should we return 0 and still reporting that error or should we just skip it at all if we cannot use mon-json?
We should silently skip this when json isn't available. That code path will only happen on very old QEMU, which presumably won't suffer from the bug you're trying to fix here. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 01/09/2015 02:15 PM, Daniel P. Berrange wrote:
On Fri, Jan 09, 2015 at 02:08:06PM +0100, Pavel Hrdina wrote:
On 01/09/2015 10:51 AM, Daniel P. Berrange wrote:
On Fri, Jan 09, 2015 at 10:02:07AM +0100, Pavel Hrdina wrote:
QEMU internally updates the size of video memory if the domain XML had provided too low memory size or there are some dependencies for a QXL devices 'vgamem' and 'ram' size. We need to know about the changes and store them into the status XML to not break migration or managedsave through different libvirt versions.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor.c | 38 ++++++++++++++++++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 ++ src/qemu/qemu_process.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 55f07f3..45bb62f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1154,6 +1154,44 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon, return ret; }
+ +/** + * To update video memory size in status XML we need to load correct values from + * QEMU. This is supported only with JSON monitor. + * + * Returns 0 on success, -1 on failure and sets proper error message. + */ +int +qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon, + virDomainVideoDefPtr video, + const char *videoName) +{ + int ret = -1; + char *path = NULL; + + if (mon->json) { + if (qemuMonitorFindObjectPath(mon, "/", videoName, &path) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to find QOM Object path for device '%s'"), + videoName); + return -1; + } + if (qemuMonitorJSONUpdateVideoMemorySize(mon, video, path) < 0) + goto cleanup; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("JSON monitor is required")); + return -1; + }
This fatal error is going to break all guest startup with non-json monitor IIUC.
Regards, Daniel
Yes I know that. Should we return 0 and still reporting that error or should we just skip it at all if we cannot use mon-json?
We should silently skip this when json isn't available. That code path will only happen on very old QEMU, which presumably won't suffer from the bug you're trying to fix here.
Regards, Daniel
OK, thanks for review. I'll send v3. Pavel
participants (2)
-
Daniel P. Berrange -
Pavel Hrdina