[PATCH] device_parsing: Use default values for vnc graphics device

libxkutil/device_parsing.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-) # HG changeset patch # User Eduardo Lima (Etrunko) <eblima@br.ibm.com> # Date 1317234028 10800 # Node ID 2313e472149a557143228949878f946278d0dd3a # Parent adc78792781448aca7a1356bd253cbdd689839cb device_parsing: Use default values for vnc graphics device This patch fixes the behavior where libvirt-cim loses the graphics device description after a call to ModifyResourceSettings method. Actually it has nothing to do with the fact that the domain is running or not. What happens is that if somehow we can't read either 'listen' or 'port' attributes, the function will fail and return immediately, skipping the device inclusion. The default values are based on the ones found in the default_graphics_device function in src/Virt_VirtualSystemManagementService.c. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -527,6 +527,17 @@ return 0; } +static char *get_attr_value_default(xmlNode *node, char *attrname, + const char *default_value) +{ + char *ret = get_attr_value(node, attrname); + + if (ret == NULL && default_value != NULL) + ret = strdup(default_value); + + return ret; +} + static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) { struct virt_device *vdev = NULL; @@ -547,13 +558,18 @@ CU_DEBUG("graphics device type = %s", gdev->type); if (STREQC(gdev->type, "vnc")) { - gdev->dev.vnc.port = get_attr_value(node, "port"); - gdev->dev.vnc.host = get_attr_value(node, "listen"); + gdev->dev.vnc.port = get_attr_value_default(node, "port", + "-1"); + gdev->dev.vnc.host = get_attr_value_default(node, "listen", + "127.0.0.1"); gdev->dev.vnc.keymap = get_attr_value(node, "keymap"); gdev->dev.vnc.passwd = get_attr_value(node, "passwd"); - - if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) + + if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) { + CU_DEBUG("Error vnc port '%p' host '%p'", + gdev->dev.vnc.port, gdev->dev.vnc.host); goto err; + } } else if (STREQC(gdev->type, "sdl")) { gdev->dev.sdl.display = get_attr_value(node, "display");

Seems fine, "-1" would let the VM itself find out which port it would use. in virsh, dumpxml could used to see the real port that vm have used, different from the definition xml with "-1", so I think in future some logic could be added if user want to see which port an active domain with "-1" port defintiion have used. Reviewed-by: Wayne Xia <xiawenc@linux.vnet.ibm.com> δΊ 2011-9-29 2:21, Eduardo Lima (Etrunko) ει:
libxkutil/device_parsing.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317234028 10800 # Node ID 2313e472149a557143228949878f946278d0dd3a # Parent adc78792781448aca7a1356bd253cbdd689839cb device_parsing: Use default values for vnc graphics device
This patch fixes the behavior where libvirt-cim loses the graphics device description after a call to ModifyResourceSettings method. Actually it has nothing to do with the fact that the domain is running or not. What happens is that if somehow we can't read either 'listen' or 'port' attributes, the function will fail and return immediately, skipping the device inclusion.
The default values are based on the ones found in the default_graphics_device function in src/Virt_VirtualSystemManagementService.c.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -527,6 +527,17 @@ return 0; }
+static char *get_attr_value_default(xmlNode *node, char *attrname, + const char *default_value) +{ + char *ret = get_attr_value(node, attrname); + + if (ret == NULL&& default_value != NULL) + ret = strdup(default_value); + + return ret; +} + static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) { struct virt_device *vdev = NULL; @@ -547,13 +558,18 @@ CU_DEBUG("graphics device type = %s", gdev->type);
if (STREQC(gdev->type, "vnc")) { - gdev->dev.vnc.port = get_attr_value(node, "port"); - gdev->dev.vnc.host = get_attr_value(node, "listen"); + gdev->dev.vnc.port = get_attr_value_default(node, "port", + "-1"); + gdev->dev.vnc.host = get_attr_value_default(node, "listen", + "127.0.0.1"); gdev->dev.vnc.keymap = get_attr_value(node, "keymap"); gdev->dev.vnc.passwd = get_attr_value(node, "passwd"); - - if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) + + if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) { + CU_DEBUG("Error vnc port '%p' host '%p'", + gdev->dev.vnc.port, gdev->dev.vnc.host); goto err; + } } else if (STREQC(gdev->type, "sdl")) { gdev->dev.sdl.display = get_attr_value(node, "display");
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Best Regards Wayne Xia mail:xiawenc@linux.vnet.ibm.com tel:86-010-82450803

+1. I like the addition of get_attr_value_default(). I suspect there are other places we could use the same approach where libvirt does not provide all the necessary defaults we rely on. On 09/28/2011 02:21 PM, Eduardo Lima (Etrunko) wrote:
libxkutil/device_parsing.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317234028 10800 # Node ID 2313e472149a557143228949878f946278d0dd3a # Parent adc78792781448aca7a1356bd253cbdd689839cb device_parsing: Use default values for vnc graphics device
This patch fixes the behavior where libvirt-cim loses the graphics device description after a call to ModifyResourceSettings method. Actually it has nothing to do with the fact that the domain is running or not. What happens is that if somehow we can't read either 'listen' or 'port' attributes, the function will fail and return immediately, skipping the device inclusion.
The default values are based on the ones found in the default_graphics_device function in src/Virt_VirtualSystemManagementService.c.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -527,6 +527,17 @@ return 0; }
+static char *get_attr_value_default(xmlNode *node, char *attrname, + const char *default_value) +{ + char *ret = get_attr_value(node, attrname); + + if (ret == NULL&& default_value != NULL) + ret = strdup(default_value); + + return ret; +} + static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) { struct virt_device *vdev = NULL; @@ -547,13 +558,18 @@ CU_DEBUG("graphics device type = %s", gdev->type);
if (STREQC(gdev->type, "vnc")) { - gdev->dev.vnc.port = get_attr_value(node, "port"); - gdev->dev.vnc.host = get_attr_value(node, "listen"); + gdev->dev.vnc.port = get_attr_value_default(node, "port", + "-1"); + gdev->dev.vnc.host = get_attr_value_default(node, "listen", + "127.0.0.1"); gdev->dev.vnc.keymap = get_attr_value(node, "keymap"); gdev->dev.vnc.passwd = get_attr_value(node, "passwd"); - - if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) + + if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) { + CU_DEBUG("Error vnc port '%p' host '%p'", + gdev->dev.vnc.port, gdev->dev.vnc.host); goto err; + } } else if (STREQC(gdev->type, "sdl")) { gdev->dev.sdl.display = get_attr_value(node, "display");
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

Pushed. On 09/28/2011 02:21 PM, Eduardo Lima (Etrunko) wrote:
libxkutil/device_parsing.c | 24 ++++++++++++++++++++---- 1 files changed, 20 insertions(+), 4 deletions(-)
# HG changeset patch # User Eduardo Lima (Etrunko)<eblima@br.ibm.com> # Date 1317234028 10800 # Node ID 2313e472149a557143228949878f946278d0dd3a # Parent adc78792781448aca7a1356bd253cbdd689839cb device_parsing: Use default values for vnc graphics device
This patch fixes the behavior where libvirt-cim loses the graphics device description after a call to ModifyResourceSettings method. Actually it has nothing to do with the fact that the domain is running or not. What happens is that if somehow we can't read either 'listen' or 'port' attributes, the function will fail and return immediately, skipping the device inclusion.
The default values are based on the ones found in the default_graphics_device function in src/Virt_VirtualSystemManagementService.c.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com>
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -527,6 +527,17 @@ return 0; }
+static char *get_attr_value_default(xmlNode *node, char *attrname, + const char *default_value) +{ + char *ret = get_attr_value(node, attrname); + + if (ret == NULL&& default_value != NULL) + ret = strdup(default_value); + + return ret; +} + static int parse_graphics_device(xmlNode *node, struct virt_device **vdevs) { struct virt_device *vdev = NULL; @@ -547,13 +558,18 @@ CU_DEBUG("graphics device type = %s", gdev->type);
if (STREQC(gdev->type, "vnc")) { - gdev->dev.vnc.port = get_attr_value(node, "port"); - gdev->dev.vnc.host = get_attr_value(node, "listen"); + gdev->dev.vnc.port = get_attr_value_default(node, "port", + "-1"); + gdev->dev.vnc.host = get_attr_value_default(node, "listen", + "127.0.0.1"); gdev->dev.vnc.keymap = get_attr_value(node, "keymap"); gdev->dev.vnc.passwd = get_attr_value(node, "passwd"); - - if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) + + if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL) { + CU_DEBUG("Error vnc port '%p' host '%p'", + gdev->dev.vnc.port, gdev->dev.vnc.host); goto err; + } } else if (STREQC(gdev->type, "sdl")) { gdev->dev.sdl.display = get_attr_value(node, "display");
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
participants (3)
-
Chip Vincent
-
Eduardo Lima (Etrunko)
-
Wayne Xia