9:42 a.m.
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.
Pavel Hrdina (3):
qemu_monitor: introduce new function to get QOM path
qemu_monitor: move qemuMonitorJSONObjectProperty from
qemu_monitor_json
qemu_process: detect updated video ram size values from QEMU
src/qemu/qemu_monitor.c | 207 ++++++++++++++++++++++++++++---------------
src/qemu/qemu_monitor.h | 38 ++++++++
src/qemu/qemu_monitor_json.c | 12 +--
src/qemu/qemu_monitor_json.h | 34 +------
src/qemu/qemu_process.c | 108 ++++++++++++++++++++++
tests/qemumonitorjsontest.c | 10 +--
6 files changed, 295 insertions(+), 114 deletions(-)
--
2.0.4
9:42 a.m.
New subject: [libvirt] [PATCH 1/3] qemu_monitor: introduce new function to get QOM path
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(a)redhat.com>
---
src/qemu/qemu_monitor.c | 173 ++++++++++++++++++++++++++++--------------------
1 file changed, 102 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c9c84f9..ee4460f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1008,22 +1008,90 @@ 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
+qemuMonitorFindObjectPath(qemuMonitorPtr mon,
+ const char *curpath,
+ const char *name,
+ char **path)
+{
+ ssize_t i, npaths = 0;
+ int ret = -2;
+ char *nextpath = NULL;
+ char *type = NULL;
+ qemuMonitorJSONListPathPtr *paths = NULL;
+
+ if (virAsprintf(&type, "link<%s>", name) < 0)
+ return -1;
+
+ VIR_DEBUG("Searching for '%s' Object Path starting at
'%s'", type, curpath);
+
+ npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
+ if (npaths < 0)
+ goto cleanup;
+
+ for (i = 0; i < npaths && ret == -2; i++) {
+
+ 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;
+ }
+
+ /* Type entries that begin with "child<" are a branch that can be
+ * traversed looking for more entries
+ */
+ if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
+ if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name)
< 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ ret = qemuMonitorFindObjectPath(mon, nextpath, name, path);
+ }
+ }
+
+ cleanup:
+ for (i = 0; i < npaths; i++)
+ qemuMonitorJSONListPathFree(paths[i]);
+ VIR_FREE(paths);
+ 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()
*/
@@ -1032,14 +1100,13 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
virDomainObjPtr vm,
const char *curpath)
{
- ssize_t i, j, npaths = 0, nprops = 0;
- int ret = 0;
- char *nextpath = NULL;
- qemuMonitorJSONListPathPtr *paths = NULL;
+ ssize_t i, nprops = 0;
+ int ret = -1;
+ char *path = NULL;
qemuMonitorJSONListPathPtr *bprops = NULL;
if (mon->balloonpath) {
- return 1;
+ return 0;
} else if (mon->ballooninit) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot determine balloon device path"));
@@ -1055,70 +1122,34 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
return -1;
}
- VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
-
- npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
- if (npaths < 0)
+ if (qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-pci",
&path) < 0)
return -1;
- for (i = 0; i < npaths && ret == 0; 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) {
- 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;
+ 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;
}
-
- /* Type entries that begin with "child<" are a branch that can be
- * traversed looking for more entries
- */
- if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
- if (virAsprintf(&nextpath, "%s/%s", curpath, paths[i]->name)
< 0) {
- ret = -1;
- goto cleanup;
- }
- ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
- }
}
+
+ /* 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 < npaths; i++)
- qemuMonitorJSONListPathFree(paths[i]);
- VIR_FREE(paths);
- for (j = 0; j < nprops; j++)
- qemuMonitorJSONListPathFree(bprops[j]);
+ for (i = 0; i < nprops; i++)
+ qemuMonitorJSONListPathFree(bprops[i]);
VIR_FREE(bprops);
- VIR_FREE(nextpath);
+ VIR_FREE(path);
return ret;
}
@@ -1660,7 +1691,7 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
return -1;
}
- if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
+ if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 0) {
ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath,
period);
}
--
2.0.4
5:54 p.m.
New subject: [libvirt] [PATCH 1/3] qemu_monitor: introduce new function to get QOM path
On 12/11/2014 10:42 AM, Pavel Hrdina wrote:
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(a)redhat.com>
---
src/qemu/qemu_monitor.c | 173 ++++++++++++++++++++++++++++--------------------
1 file changed, 102 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c9c84f9..ee4460f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1008,22 +1008,90 @@ 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
+qemuMonitorFindObjectPath(qemuMonitorPtr mon,
+ const char *curpath,
+ const char *name,
+ char **path)
+{
+ ssize_t i, npaths = 0;
+ int ret = -2;
+ char *nextpath = NULL;
+ char *type = NULL;
+ qemuMonitorJSONListPathPtr *paths = NULL;
+
+ if (virAsprintf(&type, "link<%s>", name) < 0)
+ return -1;
+
+ VIR_DEBUG("Searching for '%s' Object Path starting at
'%s'", type, curpath);
+
+ npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
+ if (npaths < 0)
+ goto cleanup;
+
+ for (i = 0; i < npaths && ret == -2; i++) {
+
+ 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;
+ }
+
+ /* Type entries that begin with "child<" are a branch that can be
+ * traversed looking for more entries
+ */
+ if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
+ if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ ret = qemuMonitorFindObjectPath(mon, nextpath, name, path);
+ }
+ }
+
+ cleanup:
+ for (i = 0; i < npaths; i++)
+ qemuMonitorJSONListPathFree(paths[i]);
+ VIR_FREE(paths);
+ VIR_FREE(nextpath);
+ VIR_FREE(type);
+ return ret;
+}
+
+
NIT: You added a /** as the comment opener previously, but not here.
+/* 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()
*/
@@ -1032,14 +1100,13 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
virDomainObjPtr vm,
const char *curpath)
{
- ssize_t i, j, npaths = 0, nprops = 0;
- int ret = 0;
- char *nextpath = NULL;
- qemuMonitorJSONListPathPtr *paths = NULL;
+ ssize_t i, nprops = 0;
+ int ret = -1;
+ char *path = NULL;
qemuMonitorJSONListPathPtr *bprops = NULL;
if (mon->balloonpath) {
- return 1;
+ return 0;
} else if (mon->ballooninit) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot determine balloon device path"));
@@ -1055,70 +1122,34 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
return -1;
}
- VIR_DEBUG("Searching for Balloon Object Path starting at %s", curpath);
-
- npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
- if (npaths < 0)
+ if (qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-pci",
&path) < 0)
return -1;
- for (i = 0; i < npaths && ret == 0; 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) {
- 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;
+ 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;
}
-
- /* Type entries that begin with "child<" are a branch that can be
- * traversed looking for more entries
- */
- if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
- if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
- ret = -1;
- goto cleanup;
- }
- ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
- }
}
+
+ /* 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 < npaths; i++)
- qemuMonitorJSONListPathFree(paths[i]);
- VIR_FREE(paths);
- for (j = 0; j < nprops; j++)
- qemuMonitorJSONListPathFree(bprops[j]);
+ for (i = 0; i < nprops; i++)
+ qemuMonitorJSONListPathFree(bprops[i]);
VIR_FREE(bprops);
- VIR_FREE(nextpath);
+ VIR_FREE(path);
return ret;
}
@@ -1660,7 +1691,7 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
return -1;
}
- if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
+ if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 0) {
A bit of a bikeshed, but the "/" could be moved into the called function
- I say this only because in patch 3 you call qemuMonitorFindObjectPath
with "/".
Could probably also just pass 'mon' and deref 'mon->vm' in the called
routine.
ACK,
John
ret = qemuMonitorJSONSetMemoryStatsPeriod(mon,
mon->balloonpath,
period);
}
1:37 a.m.
New subject: [libvirt] [PATCH 1/3] qemu_monitor: introduce new function to get QOM path
On 12/16/2014 12:54 AM, John Ferlan wrote:
On 12/11/2014 10:42 AM, Pavel Hrdina wrote:
> 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(a)redhat.com>
> ---
> src/qemu/qemu_monitor.c | 173 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 102 insertions(+), 71 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index c9c84f9..ee4460f 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1008,22 +1008,90 @@ 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
> +qemuMonitorFindObjectPath(qemuMonitorPtr mon,
> + const char *curpath,
> + const char *name,
> + char **path)
> +{
> + ssize_t i, npaths = 0;
> + int ret = -2;
> + char *nextpath = NULL;
> + char *type = NULL;
> + qemuMonitorJSONListPathPtr *paths = NULL;
> +
> + if (virAsprintf(&type, "link<%s>", name) < 0)
> + return -1;
> +
> + VIR_DEBUG("Searching for '%s' Object Path starting at
'%s'", type, curpath);
> +
> + npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
> + if (npaths < 0)
> + goto cleanup;
> +
> + for (i = 0; i < npaths && ret == -2; i++) {
> +
> + 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;
> + }
> +
> + /* Type entries that begin with "child<" are a branch that can
be
> + * traversed looking for more entries
> + */
> + if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
> + if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
> + ret = -1;
> + goto cleanup;
> + }
> +
> + ret = qemuMonitorFindObjectPath(mon, nextpath, name, path);
> + }
> + }
> +
> + cleanup:
> + for (i = 0; i < npaths; i++)
> + qemuMonitorJSONListPathFree(paths[i]);
> + VIR_FREE(paths);
> + VIR_FREE(nextpath);
> + VIR_FREE(type);
> + return ret;
> +}
> +
> +
NIT: You added a /** as the comment opener previously, but not here.
I'll fix that :)
> +/* 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()
> */
> @@ -1032,14 +1100,13 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> virDomainObjPtr vm,
> const char *curpath)
> {
> - ssize_t i, j, npaths = 0, nprops = 0;
> - int ret = 0;
> - char *nextpath = NULL;
> - qemuMonitorJSONListPathPtr *paths = NULL;
> + ssize_t i, nprops = 0;
> + int ret = -1;
> + char *path = NULL;
> qemuMonitorJSONListPathPtr *bprops = NULL;
>
> if (mon->balloonpath) {
> - return 1;
> + return 0;
> } else if (mon->ballooninit) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot determine balloon device path"));
> @@ -1055,70 +1122,34 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
> return -1;
> }
>
> - VIR_DEBUG("Searching for Balloon Object Path starting at %s",
curpath);
> -
> - npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, &paths);
> - if (npaths < 0)
> + if (qemuMonitorFindObjectPath(mon, curpath, "virtio-balloon-pci",
&path) < 0)
> return -1;
>
> - for (i = 0; i < npaths && ret == 0; 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) {
> - 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;
> + 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;
> }
> -
> - /* Type entries that begin with "child<" are a branch that can
be
> - * traversed looking for more entries
> - */
> - if (paths[i]->type && STRPREFIX(paths[i]->type,
"child<")) {
> - if (virAsprintf(&nextpath, "%s/%s", curpath,
paths[i]->name) < 0) {
> - ret = -1;
> - goto cleanup;
> - }
> - ret = qemuMonitorFindBalloonObjectPath(mon, vm, nextpath);
> - }
> }
>
> +
> + /* 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 < npaths; i++)
> - qemuMonitorJSONListPathFree(paths[i]);
> - VIR_FREE(paths);
> - for (j = 0; j < nprops; j++)
> - qemuMonitorJSONListPathFree(bprops[j]);
> + for (i = 0; i < nprops; i++)
> + qemuMonitorJSONListPathFree(bprops[i]);
> VIR_FREE(bprops);
> - VIR_FREE(nextpath);
> + VIR_FREE(path);
> return ret;
> }
>
> @@ -1660,7 +1691,7 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
> return -1;
> }
>
> - if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 1) {
> + if (qemuMonitorFindBalloonObjectPath(mon, mon->vm, "/") == 0) {
A bit of a bikeshed, but the "/" could be moved into the called function
- I say this only because in patch 3 you call qemuMonitorFindObjectPath
with "/".
The reason to have this parameter is that you can pass non root path to
speed up the search.
Could probably also just pass 'mon' and deref 'mon->vm' in the called
routine.
This is a good point, I'll fix that.
Thanks for review
Pavel
ACK,
John
> ret = qemuMonitorJSONSetMemoryStatsPeriod(mon, mon->balloonpath,
> period);
> }
>
9:42 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_monitor: move qemuMonitorJSONObjectProperty from qemu_monitor_json
Moving the struct and friends to qemu_monitor.h because the
'qemuMonitorJSONGetObjectProperty' has usage also for internal libvirt
code not only for tests.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_monitor.h | 28 ++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.c | 12 ++++++------
src/qemu/qemu_monitor_json.h | 34 ++--------------------------------
tests/qemumonitorjsontest.c | 10 +++++-----
4 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 21533a4..4918588 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -211,6 +211,34 @@ struct _qemuMonitorCallbacks {
qemuMonitorDomainSerialChangeCallback domainSerialChange;
};
+/* Flags for the 'type' field in _qemuMonitorObjectProperty */
+typedef enum {
+ QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
+ QEMU_MONITOR_OBJECT_PROPERTY_INT,
+ QEMU_MONITOR_OBJECT_PROPERTY_LONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_UINT,
+ QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
+ QEMU_MONITOR_OBJECT_PROPERTY_STRING,
+
+ QEMU_MONITOR_OBJECT_PROPERTY_LAST
+} qemuMonitorObjectPropertyType;
+
+typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
+typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
+struct _qemuMonitorObjectProperty {
+ int type; /* qemuMonitorObjectPropertyType */
+ union {
+ bool b;
+ int iv;
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
+
char *qemuMonitorEscapeArg(const char *in);
char *qemuMonitorUnescapeArg(const char *in);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 162579b..7854b9c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1489,10 +1489,10 @@ qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
char *balloonpath,
int period)
{
- qemuMonitorJSONObjectProperty prop;
+ qemuMonitorObjectProperty prop;
/* Set to the value in memballoon (could enable or disable) */
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
prop.val.iv = period;
if (qemuMonitorJSONSetObjectProperty(mon, balloonpath,
@@ -5287,7 +5287,7 @@ void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths)
int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
{
int ret;
virJSONValuePtr cmd;
@@ -5317,7 +5317,7 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
goto cleanup;
}
- switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
+ switch ((qemuMonitorObjectPropertyType) prop->type) {
/* Simple cases of boolean, int, long, uint, ulong, double, and string
* will receive return value as part of {"return": xxx} statement
*/
@@ -5378,13 +5378,13 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
{
int ret = -1;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
- switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
+ switch ((qemuMonitorObjectPropertyType) prop->type) {
/* Simple cases of boolean, int, long, uint, ulong, double, and string
* will receive return value as part of {"return": xxx} statement
*/
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ae20fb1..b8f14bc 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -388,47 +388,17 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
-/* ObjectProperty structures and Get/Set API's are public only
- * for qemumonitorjsontest
- */
-/* Flags for the 'type' field in _qemuMonitorJSONObjectProperty */
-typedef enum {
- QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
- QEMU_MONITOR_OBJECT_PROPERTY_INT,
- QEMU_MONITOR_OBJECT_PROPERTY_LONG,
- QEMU_MONITOR_OBJECT_PROPERTY_UINT,
- QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
- QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
- QEMU_MONITOR_OBJECT_PROPERTY_STRING,
-
- QEMU_MONITOR_OBJECT_PROPERTY_LAST
-} qemuMonitorJSONObjectPropertyType;
-
-typedef struct _qemuMonitorJSONObjectProperty qemuMonitorJSONObjectProperty;
-typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
-struct _qemuMonitorJSONObjectProperty {
- int type; /* qemuMonitorJSONObjectPropertyType */
- union {
- bool b;
- int iv;
- long long l;
- unsigned int ui;
- unsigned long long ul;
- double d;
- char *str;
- } val;
-};
int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index bd92e63..14cde7c 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -895,7 +895,7 @@ testQemuMonitorJSONGetObjectProperty(const void *data)
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
int ret = -1;
- qemuMonitorJSONObjectProperty prop;
+ qemuMonitorObjectProperty prop;
if (!test)
return -1;
@@ -905,7 +905,7 @@ testQemuMonitorJSONGetObjectProperty(const void *data)
goto cleanup;
/* Present with path and property */
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test),
"/machine/i440fx",
@@ -938,7 +938,7 @@ testQemuMonitorJSONSetObjectProperty(const void *data)
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
int ret = -1;
- qemuMonitorJSONObjectProperty prop;
+ qemuMonitorObjectProperty prop;
if (!test)
return -1;
@@ -951,7 +951,7 @@ testQemuMonitorJSONSetObjectProperty(const void *data)
goto cleanup;
/* Let's attempt the setting */
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
prop.val.b = true;
if (qemuMonitorJSONSetObjectProperty(qemuMonitorTestGetMonitor(test),
@@ -963,7 +963,7 @@ testQemuMonitorJSONSetObjectProperty(const void *data)
/* To make sure it worked, fetch the property - if this succeeds then
* we didn't hose things
*/
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test),
"/machine/i440fx",
--
2.0.4
5:59 p.m.
New subject: [libvirt] [PATCH 2/3] qemu_monitor: move qemuMonitorJSONObjectProperty from qemu_monitor_json
On 12/11/2014 10:42 AM, Pavel Hrdina wrote:
Moving the struct and friends to qemu_monitor.h because the
'qemuMonitorJSONGetObjectProperty' has usage also for internal libvirt
code not only for tests.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_monitor.h | 28 ++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.c | 12 ++++++------
src/qemu/qemu_monitor_json.h | 34 ++--------------------------------
tests/qemumonitorjsontest.c | 10 +++++-----
4 files changed, 41 insertions(+), 43 deletions(-)
ACK
John
1:25 a.m.
New subject: [libvirt] [PATCH 2/3] qemu_monitor: move qemuMonitorJSONObjectProperty from qemu_monitor_json
On 12/11/2014 04:42 PM, Pavel Hrdina wrote:
Moving the struct and friends to qemu_monitor.h because the
'qemuMonitorJSONGetObjectProperty' has usage also for internal libvirt
code not only for tests.
These properties are not really needed outside qemu_monitor_json.c if you keep
FindObjectPath in _json.c as well and create a wrapper like:
qemuMonitorGetVideoRam(*device, *alias, *vram, *ram, *vgamem)
Jan
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_monitor.h | 28 ++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.c | 12 ++++++------
src/qemu/qemu_monitor_json.h | 34 ++--------------------------------
tests/qemumonitorjsontest.c | 10 +++++-----
4 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 21533a4..4918588 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -211,6 +211,34 @@ struct _qemuMonitorCallbacks {
qemuMonitorDomainSerialChangeCallback domainSerialChange;
};
+/* Flags for the 'type' field in _qemuMonitorObjectProperty */
+typedef enum {
+ QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
+ QEMU_MONITOR_OBJECT_PROPERTY_INT,
+ QEMU_MONITOR_OBJECT_PROPERTY_LONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_UINT,
+ QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
+ QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
+ QEMU_MONITOR_OBJECT_PROPERTY_STRING,
+
+ QEMU_MONITOR_OBJECT_PROPERTY_LAST
+} qemuMonitorObjectPropertyType;
+
+typedef struct _qemuMonitorObjectProperty qemuMonitorObjectProperty;
+typedef qemuMonitorObjectProperty *qemuMonitorObjectPropertyPtr;
+struct _qemuMonitorObjectProperty {
+ int type; /* qemuMonitorObjectPropertyType */
+ union {
+ bool b;
+ int iv;
+ long long l;
+ unsigned int ui;
+ unsigned long long ul;
+ double d;
+ char *str;
+ } val;
+};
+
char *qemuMonitorEscapeArg(const char *in);
char *qemuMonitorUnescapeArg(const char *in);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 162579b..7854b9c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1489,10 +1489,10 @@ qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
char *balloonpath,
int period)
{
- qemuMonitorJSONObjectProperty prop;
+ qemuMonitorObjectProperty prop;
/* Set to the value in memballoon (could enable or disable) */
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT;
prop.val.iv = period;
if (qemuMonitorJSONSetObjectProperty(mon, balloonpath,
@@ -5287,7 +5287,7 @@ void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths)
int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
{
int ret;
virJSONValuePtr cmd;
@@ -5317,7 +5317,7 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
goto cleanup;
}
- switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
+ switch ((qemuMonitorObjectPropertyType) prop->type) {
/* Simple cases of boolean, int, long, uint, ulong, double, and string
* will receive return value as part of {"return": xxx} statement
*/
@@ -5378,13 +5378,13 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
{
int ret = -1;
virJSONValuePtr cmd = NULL;
virJSONValuePtr reply = NULL;
- switch ((qemuMonitorJSONObjectPropertyType) prop->type) {
+ switch ((qemuMonitorObjectPropertyType) prop->type) {
/* Simple cases of boolean, int, long, uint, ulong, double, and string
* will receive return value as part of {"return": xxx} statement
*/
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ae20fb1..b8f14bc 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -388,47 +388,17 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon,
void qemuMonitorJSONListPathFree(qemuMonitorJSONListPathPtr paths);
-/* ObjectProperty structures and Get/Set API's are public only
- * for qemumonitorjsontest
- */
-/* Flags for the 'type' field in _qemuMonitorJSONObjectProperty */
-typedef enum {
- QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN=1,
- QEMU_MONITOR_OBJECT_PROPERTY_INT,
- QEMU_MONITOR_OBJECT_PROPERTY_LONG,
- QEMU_MONITOR_OBJECT_PROPERTY_UINT,
- QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
- QEMU_MONITOR_OBJECT_PROPERTY_DOUBLE,
- QEMU_MONITOR_OBJECT_PROPERTY_STRING,
-
- QEMU_MONITOR_OBJECT_PROPERTY_LAST
-} qemuMonitorJSONObjectPropertyType;
-
-typedef struct _qemuMonitorJSONObjectProperty qemuMonitorJSONObjectProperty;
-typedef qemuMonitorJSONObjectProperty *qemuMonitorJSONObjectPropertyPtr;
-struct _qemuMonitorJSONObjectProperty {
- int type; /* qemuMonitorJSONObjectPropertyType */
- union {
- bool b;
- int iv;
- long long l;
- unsigned int ui;
- unsigned long long ul;
- double d;
- char *str;
- } val;
-};
int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
const char *path,
const char *property,
- qemuMonitorJSONObjectPropertyPtr prop)
+ qemuMonitorObjectPropertyPtr prop)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index bd92e63..14cde7c 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -895,7 +895,7 @@ testQemuMonitorJSONGetObjectProperty(const void *data)
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
int ret = -1;
- qemuMonitorJSONObjectProperty prop;
+ qemuMonitorObjectProperty prop;
if (!test)
return -1;
@@ -905,7 +905,7 @@ testQemuMonitorJSONGetObjectProperty(const void *data)
goto cleanup;
/* Present with path and property */
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test),
"/machine/i440fx",
@@ -938,7 +938,7 @@ testQemuMonitorJSONSetObjectProperty(const void *data)
virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
int ret = -1;
- qemuMonitorJSONObjectProperty prop;
+ qemuMonitorObjectProperty prop;
if (!test)
return -1;
@@ -951,7 +951,7 @@ testQemuMonitorJSONSetObjectProperty(const void *data)
goto cleanup;
/* Let's attempt the setting */
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
prop.val.b = true;
if (qemuMonitorJSONSetObjectProperty(qemuMonitorTestGetMonitor(test),
@@ -963,7 +963,7 @@ testQemuMonitorJSONSetObjectProperty(const void *data)
/* To make sure it worked, fetch the property - if this succeeds then
* we didn't hose things
*/
- memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty));
+ memset(&prop, 0, sizeof(qemuMonitorObjectProperty));
prop.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN;
if (qemuMonitorJSONGetObjectProperty(qemuMonitorTestGetMonitor(test),
"/machine/i440fx",
9:42 a.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: detect updated video ram size values from QEMU
QEMU internally updates the size of video memory if you specify too low
memory size or there are some dependencies like for QXL device and its
'vgamem' and 'ram' size. We need to know about the changes and store
them into the status XML to not break migration or managedsave trough
different libvirt versions.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_monitor.c | 34 +++++++++++++++
src/qemu/qemu_monitor.h | 10 +++++
src/qemu/qemu_process.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ee4460f..e061e02 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1153,6 +1153,40 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
return ret;
}
+
+/**
+ * For device finds its QOM Object path and after that reads and return
+ * specified property. If the path cannot be found or the QOM Object doesn't
+ * have that property it will fail and report appropriate error message.
+ *
+ * Returns 0 on success, otherwise -1.
+ */
+int
+qemuMonitorGetDeviceParam(qemuMonitorPtr mon,
+ const char *device,
+ const char *name,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ char *path = NULL;
+
+ if (qemuMonitorFindObjectPath(mon, "/", device, &path) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to find QOM Object path for device
'%s'"),
+ device);
+ return -1;
+ }
+
+ if (qemuMonitorJSONGetObjectProperty(mon, path, name, prop)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("QOM Object '%s' has no property
'%s'"), device, name);
+ VIR_FREE(path);
+ return -1;
+ }
+
+ return 0;
+}
+
+
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 4918588..dc74964 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -271,6 +271,12 @@ virJSONValuePtr qemuMonitorGetOptions(qemuMonitorPtr mon)
ATTRIBUTE_NONNULL(1);
void qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
ATTRIBUTE_NONNULL(1);
+int qemuMonitorGetDeviceParam(qemuMonitorPtr mon,
+ const char *device,
+ const char *name,
+ qemuMonitorObjectPropertyPtr prop)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4);
int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
const char *cmd,
int scm_fd,
@@ -367,6 +373,10 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
int **pids);
int qemuMonitorGetVirtType(qemuMonitorPtr mon,
int *virtType);
+int qemuMonitorGetVideoDeviceParam(qemuMonitorPtr mon,
+ const char *device,
+ const char *name,
+ qemuMonitorObjectPropertyPtr prop);
int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
unsigned long long *currmem);
int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab4df9b..551d48c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3037,6 +3037,110 @@ 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);
+ qemuMonitorObjectProperty prop = {
+ QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
+ {0}
+ };
+
+ 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 (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VGA_VGAMEM)) {
+ if (qemuMonitorGetDeviceParam(priv->mon,
+ "VGA",
+ "vgamem_mb",
+ &prop) < 0) {
+ goto error;
+ }
+
+ video->vram = prop.val.ul * 1024;
+ }
+ break;
+ case VIR_DOMAIN_VIDEO_TYPE_QXL:
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) {
+ if (qemuMonitorGetDeviceParam(priv->mon,
+ video->primary ? "qxl-vga" :
"qxl",
+ "vram_size",
+ &prop) < 0) {
+ goto error;
+ }
+
+ video->vram = prop.val.ul / 1024;
+ if (qemuMonitorGetDeviceParam(priv->mon,
+ video->primary ? "qxl-vga" :
"qxl",
+ "ram_size",
+ &prop) < 0) {
+ goto error;
+ }
+
+ video->ram = prop.val.ul / 1024;
+ if (qemuMonitorGetDeviceParam(priv->mon,
+ video->primary ? "qxl-vga" :
"qxl",
+ "vgamem_mb",
+ &prop) < 0) {
+ goto error;
+ }
+
+ video->vgamem = prop.val.ul * 1024;
+ }
+ break;
+ case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)) {
+
+ if (qemuMonitorGetDeviceParam(priv->mon,
+ "vmware-svga",
+ "vgamem_mb",
+ &prop) < 0) {
+ goto error;
+ }
+
+ 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;
+ }
+ }
+
+ qemuDomainObjExitMonitor(driver, vm);
+
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+ return -1;
+
+ return 0;
+
+ error:
+ qemuDomainObjExitMonitor(driver, vm);
+ return -1;
+}
+
+
struct qemuProcessHookData {
virConnectPtr conn;
virDomainObjPtr vm;
@@ -4787,6 +4891,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.4
7:01 p.m.
New subject: [libvirt] [PATCH 3/3] qemu_process: detect updated video ram size values from QEMU
On 12/11/2014 10:42 AM, Pavel Hrdina wrote:
QEMU internally updates the size of video memory if you specify too
low
s/you specify/the domain XML had provided a/
?Or something like that...
memory size or there are some dependencies like for QXL device and
its
s/like for/for
s/QXL device and its/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 trough
s/trough/through
different libvirt versions.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_monitor.c | 34 +++++++++++++++
src/qemu/qemu_monitor.h | 10 +++++
src/qemu/qemu_process.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 152 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ee4460f..e061e02 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1153,6 +1153,40 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
return ret;
}
+
+/**
+ * For device finds its QOM Object path and after that reads and return
+ * specified property. If the path cannot be found or the QOM Object doesn't
+ * have that property it will fail and report appropriate error message.
+ *
+ * Returns 0 on success, otherwise -1.
+ */
+int
+qemuMonitorGetDeviceParam(qemuMonitorPtr mon,
+ const char *device,
+ const char *name,
+ qemuMonitorObjectPropertyPtr prop)
+{
+ char *path = NULL;
+
+ if (qemuMonitorFindObjectPath(mon, "/", device, &path) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to find QOM Object path for device
'%s'"),
+ device);
+ return -1;
+ }
+
+ if (qemuMonitorJSONGetObjectProperty(mon, path, name, prop)) {
prop) < 0)
right?
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("QOM Object '%s' has no property
'%s'"), device, name);
+ VIR_FREE(path);
+ return -1;
+ }
[1] See below - searches through qemuMonitorFindObjectPath are somewhat
expensive (how long is the chain if we have to start at "/" each time),
but I assume that the 'prop' returned could be an array and then for
each prop<input> you'd be able to fill in the prop<output> rather than
make 'n' sequential calls.
+
VIR_FREE(path);
(strange my Coverity run didn't pick up on it, but my eyes did). Then I
thought - hey why not create two functions - the first returns path and
the second uses it as many times as there are properties to look up
instead of a prop array...
> + return 0;
> +}
+
+
> 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 4918588..dc74964 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -271,6 +271,12 @@ virJSONValuePtr qemuMonitorGetOptions(qemuMonitorPtr mon)
> ATTRIBUTE_NONNULL(1);
> void qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr options)
> ATTRIBUTE_NONNULL(1);
> +int qemuMonitorGetDeviceParam(qemuMonitorPtr mon,
> + const char *device,
> + const char *name,
> + qemuMonitorObjectPropertyPtr prop)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> + ATTRIBUTE_NONNULL(4);
> int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
> const char *cmd,
> int scm_fd,
> @@ -367,6 +373,10 @@ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
> int **pids);
> int qemuMonitorGetVirtType(qemuMonitorPtr mon,
> int *virtType);
> +int qemuMonitorGetVideoDeviceParam(qemuMonitorPtr mon,
> + const char *device,
> + const char *name,
> + qemuMonitorObjectPropertyPtr prop);
> int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon,
> unsigned long long *currmem);
> int qemuMonitorGetMemoryStats(qemuMonitorPtr mon,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index ab4df9b..551d48c 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3037,6 +3037,110 @@ 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.
> + *
So in essence, we are searching the object tree's for link<VGA>,
link<qxl-vga>/link<qxl>, or link<vmware-svga> for the specific
properties listed. Just something to add to this entry so it's easier
for the next person to understand what's going on if they choose to read
the comments...
> + * 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);
> + qemuMonitorObjectProperty prop = {
> + QEMU_MONITOR_OBJECT_PROPERTY_ULONG,
> + {0}
> + };
+
> + 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 (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VGA_VGAMEM)) {
> + if (qemuMonitorGetDeviceParam(priv->mon,
> + "VGA",
> + "vgamem_mb",
> + &prop) < 0) {
> + goto error;
> + }
+
> + video->vram = prop.val.ul * 1024;
> + }
> + break;
> + case VIR_DOMAIN_VIDEO_TYPE_QXL:
> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) {
> + if (qemuMonitorGetDeviceParam(priv->mon,
> + video->primary ?
"qxl-vga" : "qxl",
> + "vram_size",
> + &prop) < 0) {
> + goto error;
> + }
+
> + video->vram = prop.val.ul / 1024;
> + if (qemuMonitorGetDeviceParam(priv->mon,
> + video->primary ?
"qxl-vga" : "qxl",
> + "ram_size",
> + &prop) < 0) {
> + goto error;
> + }
+
> + video->ram = prop.val.ul / 1024;
> + if (qemuMonitorGetDeviceParam(priv->mon,
> + video->primary ?
"qxl-vga" : "qxl",
> + "vgamem_mb",
> + &prop) < 0) {
> + goto error;
> + }
+
> + video->vgamem = prop.val.ul * 1024;
Three trips through qemuMonitorFindObjectPath is "expensive"... This is
where it might be worthwhile to generate an array of prop's:
prop[0] = "vram_size"
prop[1] = "ram_size"
prop[2] = "vgamem_mb"
Which could then be used in the qemuMonitorGetDeviceParam*s*() to return
each data point desired.
or
return path from the first call and make 'n' calls using that path to
get properties. Although I like the idea of an array better.
> + }
> + break;
> + case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)) {
+
> + if
(qemuMonitorGetDeviceParam(priv->mon,
> + "vmware-svga",
> + "vgamem_mb",
> + &prop) < 0) {
> + goto error;
> + }
+
> + 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;
> + }
> + }
We could get here without changing anything.... Either we don't have the
specific CAP available, the value is the same as we already have stored,
or we fall into the last group of case's. In any of those cases the
subsequent save is only ripe for unnecessary errors... Maybe it's best
to have some sort of boolean that indicates a value was changed.
+
> + qemuDomainObjExitMonitor(driver, vm);
+
> + if (virDomainSaveStatus(driver->xmlopt,
cfg->stateDir, vm) < 0)
There's a few places in qemu_driver which do a VIR_WARN("Failed to save
status on vm %s", vm->def->name); if this fails... Not sure if you want
to follow that here too.
> + return -1;
+
> + return 0;
+
> + error:
> + qemuDomainObjExitMonitor(driver, vm);
> + return -1;
> +}
+
+
> struct qemuProcessHookData
{
> virConnectPtr conn;
> virDomainObjPtr vm;
> @@ -4787,6 +4891,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;
+
This only works if mon->json, correct? So if we don't have that, then
we're dead in the water?
Currently we're not dead in the water, incorrect values perhaps
displayed/used, but we're not failing to start up.
Is that what we is desired?
I think overall it looks ok, there's a couple of adjustments though and
things to understand better before an ACK though...
John
if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) {
VIR_DEBUG("Starting domain CPUs");
/* Allow the CPUS to start executing */
3608
days inactive
3633
days old
8 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Pavel Hrdina