8:36 a.m.
This are some additional features to libxl driver. Some of them require change
in domain config structures/syntax. Details described with each patch.
I've dropped patches already applied in this version.
Patches for stubdom support in xenconfig and some more tests will be later.
Marek Marczykowski-Górecki (8):
libxl: add tablet/mouse input device support
xenconfig: add support for multiple USB devices syntax
tests: xenconfig: test for multiple USB devices and other HVM options
conf: add virDomainHasNet
libxl: prevent attaching multiple netdevs with the same MAC
libxl: support domain config modification in virDomainRestoreFlags
libxl: Stubdom emulator type
libxl: pass cmdline to HVM guests
tests: add some simple libxl XML->XML tests
docs/formatdomain.html.in | 13 ++
docs/schemas/domaincommon.rng | 10 ++
src/conf/domain_conf.c | 35 ++++
src/conf/domain_conf.h | 4 +-
src/libvirt_private.syms | 1 +
src/libxl/libxl_conf.c | 60 +++++++
src/libxl/libxl_driver.c | 32 +++-
src/xenconfig/xen_common.c | 66 -------
src/xenconfig/xen_xl.c | 127 ++++++++++++++
src/xenconfig/xen_xm.c | 72 ++++++++
tests/Makefile.am | 9 +-
tests/domainschematest | 2 +-
tests/xlconfigdata/test-fullvirt-multiusb.cfg | 29 ++++
tests/xlconfigdata/test-fullvirt-multiusb.xml | 48 ++++++
tests/xlconfigtest.c | 1 +
tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml | 42 +++++
tests/xlxml2xmldata/xlxml2xml-hvm.xml | 40 +++++
tests/xlxml2xmldata/xlxml2xml-network-bridged.xml | 38 +++++
.../xlxml2xml-network-driver-domain.xml | 39 +++++
tests/xlxml2xmldata/xlxml2xml-network-routed.xml | 39 +++++
tests/xlxml2xmldata/xlxml2xml-pv.xml | 38 +++++
tests/xlxml2xmltest.c | 189 +++++++++++++++++++++
tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 4 +-
tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 4 +-
24 files changed, 863 insertions(+), 79 deletions(-)
create mode 100755 tests/xlconfigdata/test-fullvirt-multiusb.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-multiusb.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-hvm.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-network-bridged.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-network-driver-domain.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-network-routed.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-pv.xml
create mode 100644 tests/xlxml2xmltest.c
--
2.1.0
8:36 a.m.
New subject: [libvirt] [PATCH 1/9] libxl: add tablet/mouse input device support
From: Marek Marczykowski <marmarek(a)invisiblethingslab.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
Changes in v2:
- rebase on 1.2.12+
- multiple devices support
Changes in v3:
- reduce code duplication
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 2321660..50ef9d8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -749,6 +749,50 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
libxl_defbool_set(&b_info->u.hvm.vnc.enable, 0);
libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0);
+ if (def->ninputs) {
+ for (i = 0; i < def->ninputs; i++) {
+ if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("libxenlight supports only USB input"));
+ return -1;
+ }
+ }
+#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < 0)
+ return -1;
+#else
+ if (def->ninputs > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("libxenlight supports only one input device"));
+ return -1;
+ }
+#endif
+ for (i = 0; i < def->ninputs; i++) {
+ char **usbdevice;
+#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ usbdevice = &b_info->u.hvm.usbdevice_list[i];
+#else
+ usbdevice = &b_info->u.hvm.usbdevice;
+#endif
+ switch (def->inputs[i]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ VIR_FREE(*usbdevice);
+ if (VIR_STRDUP(*usbdevice, "mouse") < 0)
+ return -1;
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ VIR_FREE(*usbdevice);
+ if (VIR_STRDUP(*usbdevice, "tablet") < 0)
+ return -1;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Unknown input device type"));
+ return -1;
+ }
+ }
+ }
+
/*
* The following comment and calculation were taken directly from
* libxenlight's internal function libxl_get_required_shadow_memory():
--
2.1.0
11:25 a.m.
New subject: [libvirt] [PATCH 1/9] libxl: add tablet/mouse input device support
Marek Marczykowski-Górecki wrote:
From: Marek Marczykowski <marmarek(a)invisiblethingslab.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
Changes in v2:
- rebase on 1.2.12+
- multiple devices support
Changes in v3:
- reduce code duplication
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 2321660..50ef9d8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -749,6 +749,50 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
libxl_defbool_set(&b_info->u.hvm.vnc.enable, 0);
libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0);
+ if (def->ninputs) {
+ for (i = 0; i < def->ninputs; i++) {
+ if (def->inputs[i]->bus != VIR_DOMAIN_INPUT_BUS_USB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("libxenlight supports only USB input"));
+ return -1;
+ }
+ }
+#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ if (VIR_ALLOC_N(b_info->u.hvm.usbdevice_list, def->ninputs+1) < 0)
+ return -1;
+#else
+ if (def->ninputs > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("libxenlight supports only one input device"));
+ return -1;
+ }
+#endif
+ for (i = 0; i < def->ninputs; i++) {
+ char **usbdevice;
+#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ usbdevice = &b_info->u.hvm.usbdevice_list[i];
+#else
+ usbdevice = &b_info->u.hvm.usbdevice;
+#endif
+ switch (def->inputs[i]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ VIR_FREE(*usbdevice);
+ if (VIR_STRDUP(*usbdevice, "mouse") < 0)
+ return -1;
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ VIR_FREE(*usbdevice);
+ if (VIR_STRDUP(*usbdevice, "tablet") < 0)
+ return -1;
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Unknown input device type"));
+ return -1;
+ }
+ }
+ }
+
Looks good now; ACK.
Regards,
Jim
8:36 a.m.
New subject: [libvirt] [PATCH 2/9] xenconfig: add support for multiple USB devices syntax
In Xen>=4.3, libxl supports new syntax for USB devices:
usbdevice=[ "DEVICE", "DEVICE", ... ]
Add support for that in xenconfig driver. When only one device is
defined, keep using old syntax for backward compatibility.
Adjust tests for changed options order.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/xenconfig/xen_common.c | 66 -------------
src/xenconfig/xen_xl.c | 127 +++++++++++++++++++++++++
src/xenconfig/xen_xm.c | 72 ++++++++++++++
tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 4 +-
tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 4 +-
5 files changed, 203 insertions(+), 70 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index a2a1474..079f77d 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -972,33 +972,6 @@ xenParseEmulatedDevices(virConfPtr conf, virDomainDefPtr def)
if (str &&
xenParseSxprSound(def, str) < 0)
return -1;
-
- if (xenConfigGetString(conf, "usbdevice", &str, NULL) < 0)
- return -1;
-
- if (str &&
- (STREQ(str, "tablet") ||
- STREQ(str, "mouse") ||
- STREQ(str, "keyboard"))) {
- virDomainInputDefPtr input;
- if (VIR_ALLOC(input) < 0)
- return -1;
-
- input->bus = VIR_DOMAIN_INPUT_BUS_USB;
- if (STREQ(str, "mouse"))
- input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
- else if (STREQ(str, "tablet"))
- input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
- else if (STREQ(str, "keyboard"))
- input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
- if (VIR_ALLOC_N(def->inputs, 1) < 0) {
- virDomainInputDefFree(input);
- return -1;
-
- }
- def->inputs[0] = input;
- def->ninputs = 1;
- }
}
return 0;
@@ -1949,42 +1922,6 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def)
}
-static int
-xenFormatInputDevs(virConfPtr conf, virDomainDefPtr def)
-{
- size_t i;
-
- if (STREQ(def->os.type, "hvm")) {
- for (i = 0; i < def->ninputs; i++) {
- if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
- if (xenConfigSetInt(conf, "usb", 1) < 0)
- return -1;
-
- switch (def->inputs[i]->type) {
- case VIR_DOMAIN_INPUT_TYPE_MOUSE:
- if (xenConfigSetString(conf, "usbdevice",
"mouse") < 0)
- return -1;
-
- break;
- case VIR_DOMAIN_INPUT_TYPE_TABLET:
- if (xenConfigSetString(conf, "usbdevice",
"tablet") < 0)
- return -1;
-
- break;
- case VIR_DOMAIN_INPUT_TYPE_KBD:
- if (xenConfigSetString(conf, "usbdevice",
"keyboard") < 0)
- return -1;
-
- break;
- }
- break;
- }
- }
- }
-
- return 0;
-}
-
static int
xenFormatVif(virConfPtr conf,
@@ -2059,9 +1996,6 @@ xenFormatConfigCommon(virConfPtr conf,
if (xenFormatEmulator(conf, def) < 0)
return -1;
- if (xenFormatInputDevs(conf, def) < 0)
- return -1;
-
if (xenFormatVfb(conf, def, xendConfigVersion) < 0)
return -1;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 95ef5f4..f127ebe 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -289,6 +289,59 @@ xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
goto cleanup;
}
+static int
+xenParseXLInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+ virConfValuePtr val;
+
+ if (STREQ(def->os.type, "hvm")) {
+ val = virConfGetValue(conf, "usbdevice");
+ /* usbdevice can be defined as either a single string or a list */
+ if (val && val->type == VIR_CONF_LIST) {
+#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ val = val->list;
+#else
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("multiple USB devices not supported"));
+ return -1;
+#endif
+ }
+ /* otherwise val->next is NULL, so can be handled by the same code */
+ while (val) {
+ if (val->type != VIR_CONF_STRING) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("config value %s was malformed"),
+ "usbdevice");
+ return -1;
+ }
+ str = val->str;
+
+ if (str &&
+ (STREQ(str, "tablet") ||
+ STREQ(str, "mouse") ||
+ STREQ(str, "keyboard"))) {
+ virDomainInputDefPtr input;
+ if (VIR_ALLOC(input) < 0)
+ return -1;
+
+ input->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ if (STREQ(str, "mouse"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+ else if (STREQ(str, "tablet"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+ else if (STREQ(str, "keyboard"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
+ if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
+ virDomainInputDefFree(input);
+ return -1;
+ }
+ }
+ val = val->next;
+ }
+ }
+ return 0;
+}
virDomainDefPtr
xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
@@ -310,6 +363,9 @@ xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
if (xenParseXLSpice(conf, def) < 0)
goto cleanup;
+ if (xenParseXLInputDevs(conf, def) < 0)
+ goto cleanup;
+
return def;
cleanup:
@@ -488,6 +544,74 @@ xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int
+xenFormatXLInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ size_t i;
+ const char *devtype;
+ virConfValuePtr usbdevices = NULL, lastdev;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (VIR_ALLOC(usbdevices) < 0)
+ goto error;
+
+ usbdevices->type = VIR_CONF_LIST;
+ usbdevices->list = NULL;
+ lastdev = NULL;
+ for (i = 0; i < def->ninputs; i++) {
+ if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
+ if (xenConfigSetInt(conf, "usb", 1) < 0)
+ goto error;
+
+ switch (def->inputs[i]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ devtype = "mouse";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ devtype = "tablet";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_KBD:
+ devtype = "keyboard";
+ break;
+ default:
+ continue;
+ }
+
+ if (lastdev == NULL) {
+ if (VIR_ALLOC(lastdev) < 0)
+ goto error;
+ usbdevices->list = lastdev;
+ } else {
+ if (VIR_ALLOC(lastdev->next) < 0)
+ goto error;
+ lastdev = lastdev->next;
+ }
+ lastdev->type = VIR_CONF_STRING;
+ if (VIR_STRDUP(lastdev->str, devtype) < 0)
+ goto error;
+ }
+ }
+ if (usbdevices->list != NULL) {
+ if (usbdevices->list->next == NULL) {
+ /* for compatibility with Xen <= 4.2, use old syntax when
+ * only one device present */
+ if (xenConfigSetString(conf, "usbdevice",
usbdevices->list->str) < 0)
+ goto error;
+ virConfFreeValue(usbdevices);
+ } else {
+ virConfSetValue(conf, "usbdevice", usbdevices);
+ }
+ } else {
+ VIR_FREE(usbdevices);
+ }
+ }
+
+ return 0;
+ error:
+ virConfFreeValue(usbdevices);
+ return -1;
+}
+
virConfPtr
xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
@@ -506,6 +630,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int
xendConfigVersion)
if (xenFormatXLSpice(conf, def) < 0)
goto cleanup;
+ if (xenFormatXLInputDevs(conf, def) < 0)
+ goto cleanup;
+
return conf;
cleanup:
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 2f57cd2..5d70af6 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -365,6 +365,38 @@ xenFormatXMDisks(virConfPtr conf, virDomainDefPtr def, int
xendConfigVersion)
}
+static int
+xenParseXMInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (xenConfigGetString(conf, "usbdevice", &str, NULL) < 0)
+ return -1;
+ if (str &&
+ (STREQ(str, "tablet") ||
+ STREQ(str, "mouse") ||
+ STREQ(str, "keyboard"))) {
+ virDomainInputDefPtr input;
+ if (VIR_ALLOC(input) < 0)
+ return -1;
+
+ input->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ if (STREQ(str, "mouse"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+ else if (STREQ(str, "tablet"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+ else if (STREQ(str, "keyboard"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
+ if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
+ virDomainInputDefFree(input);
+ return -1;
+ }
+ }
+ }
+ return 0;
+}
+
/*
* Convert an XM config record into a virDomainDef object.
*/
@@ -387,6 +419,9 @@ xenParseXM(virConfPtr conf,
if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
goto cleanup;
+ if (xenParseXMInputDevs(conf, def) < 0)
+ goto cleanup;
+
return def;
cleanup:
@@ -394,6 +429,40 @@ xenParseXM(virConfPtr conf,
return NULL;
}
+static int
+xenFormatXMInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ size_t i;
+ const char *devtype;
+
+ if (STREQ(def->os.type, "hvm")) {
+ for (i = 0; i < def->ninputs; i++) {
+ if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
+ if (xenConfigSetInt(conf, "usb", 1) < 0)
+ return -1;
+
+ switch (def->inputs[i]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ devtype = "mouse";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ devtype = "tablet";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_KBD:
+ devtype = "keyboard";
+ break;
+ default:
+ continue;
+ }
+ if (xenConfigSetString(conf, "usbdevice", devtype) < 0)
+ return -1;
+ break;
+ }
+ }
+ }
+ return 0;
+}
+
/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
either 32, or 64 on a platform where long is big enough. */
@@ -418,6 +487,9 @@ xenFormatXM(virConnectPtr conn,
if (xenFormatXMDisks(conf, def, xendConfigVersion) < 0)
goto cleanup;
+ if (xenFormatXMInputDevs(conf, def) < 0)
+ goto cleanup;
+
return conf;
cleanup:
diff --git a/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
b/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
index b24d7a4..7b252cf 100755
--- a/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
+++ b/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
@@ -14,8 +14,6 @@ on_poweroff = "destroy"
on_reboot = "restart"
on_crash = "restart"
device_model = "/usr/lib/xen/bin/qemu-dm"
-usb = 1
-usbdevice = "mouse"
sdl = 0
vnc = 1
vncunused = 1
@@ -25,3 +23,5 @@ vif = [
"mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=
parallel = "none"
serial = "none"
disk = [ "phy:/dev/HostVG/XenGuest2,hda,w",
"file:/root/boot.iso,hdc:cdrom,r" ]
+usb = 1
+usbdevice = "mouse"
diff --git a/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
b/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
index 189cd15..7e6cd36 100755
--- a/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
+++ b/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
@@ -14,8 +14,6 @@ on_poweroff = "destroy"
on_reboot = "restart"
on_crash = "restart"
device_model = "/usr/lib/xen/bin/qemu-dm"
-usb = 1
-usbdevice = "tablet"
sdl = 0
vnc = 1
vncunused = 1
@@ -25,3 +23,5 @@ vif = [
"mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=
parallel = "none"
serial = "none"
disk = [ "phy:/dev/HostVG/XenGuest2,hda,w",
"file:/root/boot.iso,hdc:cdrom,r" ]
+usb = 1
+usbdevice = "tablet"
--
2.1.0
12:03 p.m.
New subject: [libvirt] [PATCH 2/9] xenconfig: add support for multiple USB devices syntax
Marek Marczykowski-Górecki wrote:
In Xen>=4.3, libxl supports new syntax for USB devices:
usbdevice=[ "DEVICE", "DEVICE", ... ]
Add support for that in xenconfig driver. When only one device is
defined, keep using old syntax for backward compatibility.
Adjust tests for changed options order.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/xenconfig/xen_common.c | 66 -------------
src/xenconfig/xen_xl.c | 127 +++++++++++++++++++++++++
src/xenconfig/xen_xm.c | 72 ++++++++++++++
tests/xmconfigdata/test-fullvirt-usbmouse.cfg | 4 +-
tests/xmconfigdata/test-fullvirt-usbtablet.cfg | 4 +-
5 files changed, 203 insertions(+), 70 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index a2a1474..079f77d 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -972,33 +972,6 @@ xenParseEmulatedDevices(virConfPtr conf, virDomainDefPtr def)
if (str &&
xenParseSxprSound(def, str) < 0)
return -1;
-
- if (xenConfigGetString(conf, "usbdevice", &str, NULL) < 0)
- return -1;
-
- if (str &&
- (STREQ(str, "tablet") ||
- STREQ(str, "mouse") ||
- STREQ(str, "keyboard"))) {
- virDomainInputDefPtr input;
- if (VIR_ALLOC(input) < 0)
- return -1;
-
- input->bus = VIR_DOMAIN_INPUT_BUS_USB;
- if (STREQ(str, "mouse"))
- input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
- else if (STREQ(str, "tablet"))
- input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
- else if (STREQ(str, "keyboard"))
- input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
- if (VIR_ALLOC_N(def->inputs, 1) < 0) {
- virDomainInputDefFree(input);
- return -1;
-
- }
- def->inputs[0] = input;
- def->ninputs = 1;
- }
}
return 0;
@@ -1949,42 +1922,6 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def)
}
-static int
-xenFormatInputDevs(virConfPtr conf, virDomainDefPtr def)
-{
- size_t i;
-
- if (STREQ(def->os.type, "hvm")) {
- for (i = 0; i < def->ninputs; i++) {
- if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
- if (xenConfigSetInt(conf, "usb", 1) < 0)
- return -1;
-
- switch (def->inputs[i]->type) {
- case VIR_DOMAIN_INPUT_TYPE_MOUSE:
- if (xenConfigSetString(conf, "usbdevice",
"mouse") < 0)
- return -1;
-
- break;
- case VIR_DOMAIN_INPUT_TYPE_TABLET:
- if (xenConfigSetString(conf, "usbdevice",
"tablet") < 0)
- return -1;
-
- break;
- case VIR_DOMAIN_INPUT_TYPE_KBD:
- if (xenConfigSetString(conf, "usbdevice",
"keyboard") < 0)
- return -1;
-
- break;
- }
- break;
- }
- }
- }
-
- return 0;
-}
-
static int
xenFormatVif(virConfPtr conf,
@@ -2059,9 +1996,6 @@ xenFormatConfigCommon(virConfPtr conf,
if (xenFormatEmulator(conf, def) < 0)
return -1;
- if (xenFormatInputDevs(conf, def) < 0)
- return -1;
-
if (xenFormatVfb(conf, def, xendConfigVersion) < 0)
return -1;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 95ef5f4..f127ebe 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -289,6 +289,59 @@ xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
goto cleanup;
}
+static int
+xenParseXLInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+ virConfValuePtr val;
+
+ if (STREQ(def->os.type, "hvm")) {
+ val = virConfGetValue(conf, "usbdevice");
+ /* usbdevice can be defined as either a single string or a list */
+ if (val && val->type == VIR_CONF_LIST) {
+#ifdef LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ val = val->list;
+#else
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("multiple USB devices not supported"));
+ return -1;
+#endif
+ }
+ /* otherwise val->next is NULL, so can be handled by the same code */
+ while (val) {
+ if (val->type != VIR_CONF_STRING) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("config value %s was malformed"),
+ "usbdevice");
+ return -1;
+ }
+ str = val->str;
+
+ if (str &&
+ (STREQ(str, "tablet") ||
+ STREQ(str, "mouse") ||
+ STREQ(str, "keyboard"))) {
+ virDomainInputDefPtr input;
+ if (VIR_ALLOC(input) < 0)
+ return -1;
+
+ input->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ if (STREQ(str, "mouse"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+ else if (STREQ(str, "tablet"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+ else if (STREQ(str, "keyboard"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
+ if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0)
{
+ virDomainInputDefFree(input);
+ return -1;
+ }
+ }
+ val = val->next;
+ }
+ }
+ return 0;
+}
virDomainDefPtr
xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
@@ -310,6 +363,9 @@ xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion)
if (xenParseXLSpice(conf, def) < 0)
goto cleanup;
+ if (xenParseXLInputDevs(conf, def) < 0)
+ goto cleanup;
+
return def;
cleanup:
@@ -488,6 +544,74 @@ xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
return 0;
}
+static int
+xenFormatXLInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ size_t i;
+ const char *devtype;
+ virConfValuePtr usbdevices = NULL, lastdev;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (VIR_ALLOC(usbdevices) < 0)
+ goto error;
+
+ usbdevices->type = VIR_CONF_LIST;
+ usbdevices->list = NULL;
+ lastdev = NULL;
+ for (i = 0; i < def->ninputs; i++) {
+ if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
+ if (xenConfigSetInt(conf, "usb", 1) < 0)
+ goto error;
+
+ switch (def->inputs[i]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ devtype = "mouse";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ devtype = "tablet";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_KBD:
+ devtype = "keyboard";
+ break;
+ default:
+ continue;
+ }
+
+ if (lastdev == NULL) {
+ if (VIR_ALLOC(lastdev) < 0)
+ goto error;
+ usbdevices->list = lastdev;
+ } else {
+ if (VIR_ALLOC(lastdev->next) < 0)
+ goto error;
+ lastdev = lastdev->next;
+ }
+ lastdev->type = VIR_CONF_STRING;
+ if (VIR_STRDUP(lastdev->str, devtype) < 0)
+ goto error;
+ }
+ }
+ if (usbdevices->list != NULL) {
+ if (usbdevices->list->next == NULL) {
+ /* for compatibility with Xen <= 4.2, use old syntax when
+ * only one device present */
+ if (xenConfigSetString(conf, "usbdevice",
usbdevices->list->str) < 0)
+ goto error;
+ virConfFreeValue(usbdevices);
+ } else {
+ virConfSetValue(conf, "usbdevice", usbdevices);
+ }
+ } else {
+ VIR_FREE(usbdevices);
+ }
+ }
+
+ return 0;
+ error:
+ virConfFreeValue(usbdevices);
+ return -1;
I had to stare at this a bit to convince myself memory was being freed
properly. xenConfigSetString copies the string, so need virConfFreeValue
afterwards - check. virtConfSetValue takes ownership of value, so no
need to free - check. Cleanup on error - check. Caller frees values
when conf is destoryed - check. Coverity, and subsequently John, will
complain loudly if I've missed a check.
ACK.
Regards,
Jim
+}
+
virConfPtr
xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion)
@@ -506,6 +630,9 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int
xendConfigVersion)
if (xenFormatXLSpice(conf, def) < 0)
goto cleanup;
+ if (xenFormatXLInputDevs(conf, def) < 0)
+ goto cleanup;
+
return conf;
cleanup:
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 2f57cd2..5d70af6 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -365,6 +365,38 @@ xenFormatXMDisks(virConfPtr conf, virDomainDefPtr def, int
xendConfigVersion)
}
+static int
+xenParseXMInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ const char *str;
+
+ if (STREQ(def->os.type, "hvm")) {
+ if (xenConfigGetString(conf, "usbdevice", &str, NULL) < 0)
+ return -1;
+ if (str &&
+ (STREQ(str, "tablet") ||
+ STREQ(str, "mouse") ||
+ STREQ(str, "keyboard"))) {
+ virDomainInputDefPtr input;
+ if (VIR_ALLOC(input) < 0)
+ return -1;
+
+ input->bus = VIR_DOMAIN_INPUT_BUS_USB;
+ if (STREQ(str, "mouse"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+ else if (STREQ(str, "tablet"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+ else if (STREQ(str, "keyboard"))
+ input->type = VIR_DOMAIN_INPUT_TYPE_KBD;
+ if (VIR_APPEND_ELEMENT(def->inputs, def->ninputs, input) < 0) {
+ virDomainInputDefFree(input);
+ return -1;
+ }
+ }
+ }
+ return 0;
+}
+
/*
* Convert an XM config record into a virDomainDef object.
*/
@@ -387,6 +419,9 @@ xenParseXM(virConfPtr conf,
if (xenParseXMDisk(conf, def, xendConfigVersion) < 0)
goto cleanup;
+ if (xenParseXMInputDevs(conf, def) < 0)
+ goto cleanup;
+
return def;
cleanup:
@@ -394,6 +429,40 @@ xenParseXM(virConfPtr conf,
return NULL;
}
+static int
+xenFormatXMInputDevs(virConfPtr conf, virDomainDefPtr def)
+{
+ size_t i;
+ const char *devtype;
+
+ if (STREQ(def->os.type, "hvm")) {
+ for (i = 0; i < def->ninputs; i++) {
+ if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) {
+ if (xenConfigSetInt(conf, "usb", 1) < 0)
+ return -1;
+
+ switch (def->inputs[i]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ devtype = "mouse";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ devtype = "tablet";
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_KBD:
+ devtype = "keyboard";
+ break;
+ default:
+ continue;
+ }
+ if (xenConfigSetString(conf, "usbdevice", devtype) < 0)
+ return -1;
+ break;
+ }
+ }
+ }
+ return 0;
+}
+
/* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is
either 32, or 64 on a platform where long is big enough. */
@@ -418,6 +487,9 @@ xenFormatXM(virConnectPtr conn,
if (xenFormatXMDisks(conf, def, xendConfigVersion) < 0)
goto cleanup;
+ if (xenFormatXMInputDevs(conf, def) < 0)
+ goto cleanup;
+
return conf;
cleanup:
diff --git a/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
b/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
index b24d7a4..7b252cf 100755
--- a/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
+++ b/tests/xmconfigdata/test-fullvirt-usbmouse.cfg
@@ -14,8 +14,6 @@ on_poweroff = "destroy"
on_reboot = "restart"
on_crash = "restart"
device_model = "/usr/lib/xen/bin/qemu-dm"
-usb = 1
-usbdevice = "mouse"
sdl = 0
vnc = 1
vncunused = 1
@@ -25,3 +23,5 @@ vif = [
"mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=
parallel = "none"
serial = "none"
disk = [ "phy:/dev/HostVG/XenGuest2,hda,w",
"file:/root/boot.iso,hdc:cdrom,r" ]
+usb = 1
+usbdevice = "mouse"
diff --git a/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
b/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
index 189cd15..7e6cd36 100755
--- a/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
+++ b/tests/xmconfigdata/test-fullvirt-usbtablet.cfg
@@ -14,8 +14,6 @@ on_poweroff = "destroy"
on_reboot = "restart"
on_crash = "restart"
device_model = "/usr/lib/xen/bin/qemu-dm"
-usb = 1
-usbdevice = "tablet"
sdl = 0
vnc = 1
vncunused = 1
@@ -25,3 +23,5 @@ vif = [
"mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=
parallel = "none"
serial = "none"
disk = [ "phy:/dev/HostVG/XenGuest2,hda,w",
"file:/root/boot.iso,hdc:cdrom,r" ]
+usb = 1
+usbdevice = "tablet"
8:36 a.m.
New subject: [libvirt] [PATCH 3/9] tests: xenconfig: test for multiple USB devices and other HVM options
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
tests/xlconfigdata/test-fullvirt-multiusb.cfg | 29 ++++++++++++++++
tests/xlconfigdata/test-fullvirt-multiusb.xml | 48 +++++++++++++++++++++++++++
tests/xlconfigtest.c | 1 +
3 files changed, 78 insertions(+)
create mode 100755 tests/xlconfigdata/test-fullvirt-multiusb.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-multiusb.xml
diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.cfg
b/tests/xlconfigdata/test-fullvirt-multiusb.cfg
new file mode 100755
index 0000000..ba4bf52
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-multiusb.cfg
@@ -0,0 +1,29 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader"
+boot = "d"
+pae = 1
+acpi = 1
+apic = 1
+hap = 0
+viridian = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-dm"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+vncpasswd = "123poi"
+vif = [
"mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu"
]
+parallel = "none"
+serial = "none"
+disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy",
"/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ]
+usb = 1
+usbdevice = [ "mouse", "tablet" ]
diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.xml
b/tests/xlconfigdata/test-fullvirt-multiusb.xml
new file mode 100644
index 0000000..642c242
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-multiusb.xml
@@ -0,0 +1,48 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc' adjustment='reset'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+ <disk type='block' device='disk'>
+ <driver name='phy' type='raw'/>
+ <source dev='/dev/HostVG/XenGuest2'/>
+ <target dev='hda' bus='ide'/>
+ </disk>
+ <disk type='file' device='cdrom'>
+ <driver name='qemu' type='raw'/>
+ <source file='/root/boot.iso'/>
+ <target dev='hdc' bus='ide'/>
+ <readonly/>
+ </disk>
+ <interface type='bridge'>
+ <mac address='00:16:3e:66:92:9c'/>
+ <source bridge='xenbr1'/>
+ <script path='vif-bridge'/>
+ <model type='e1000'/>
+ </interface>
+ <input type='mouse' bus='usb'/>
+ <input type='tablet' bus='usb'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'
listen='127.0.0.1' passwd='123poi'>
+ <listen type='address' address='127.0.0.1'/>
+ </graphics>
+ </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 8c4c82c..6d4aa6d 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -215,6 +215,7 @@ mymain(void)
DO_TEST("new-disk", 3);
DO_TEST("spice", 3);
+ DO_TEST("fullvirt-multiusb", 3);
virObjectUnref(caps);
virObjectUnref(xmlopt);
--
2.1.0
12:06 p.m.
New subject: [libvirt] [PATCH 3/9] tests: xenconfig: test for multiple USB devices and other HVM options
Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
tests/xlconfigdata/test-fullvirt-multiusb.cfg | 29 ++++++++++++++++
tests/xlconfigdata/test-fullvirt-multiusb.xml | 48 +++++++++++++++++++++++++++
tests/xlconfigtest.c | 1 +
3 files changed, 78 insertions(+)
create mode 100755 tests/xlconfigdata/test-fullvirt-multiusb.cfg
create mode 100644 tests/xlconfigdata/test-fullvirt-multiusb.xml
Nice, thanks! I've tested this with the various combinations of
--with{out}-xen --with{out}-libxl configure options.
ACK.
Regards,
Jim
diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.cfg
b/tests/xlconfigdata/test-fullvirt-multiusb.cfg
new file mode 100755
index 0000000..ba4bf52
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-multiusb.cfg
@@ -0,0 +1,29 @@
+name = "XenGuest2"
+uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
+maxmem = 579
+memory = 394
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader"
+boot = "d"
+pae = 1
+acpi = 1
+apic = 1
+hap = 0
+viridian = 0
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "restart"
+on_crash = "restart"
+device_model = "/usr/lib/xen/bin/qemu-dm"
+sdl = 0
+vnc = 1
+vncunused = 1
+vnclisten = "127.0.0.1"
+vncpasswd = "123poi"
+vif = [
"mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu"
]
+parallel = "none"
+serial = "none"
+disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,backendtype=phy",
"/root/boot.iso,raw,hdc,r,backendtype=qdisk,devtype=cdrom" ]
+usb = 1
+usbdevice = [ "mouse", "tablet" ]
diff --git a/tests/xlconfigdata/test-fullvirt-multiusb.xml
b/tests/xlconfigdata/test-fullvirt-multiusb.xml
new file mode 100644
index 0000000..642c242
--- /dev/null
+++ b/tests/xlconfigdata/test-fullvirt-multiusb.xml
@@ -0,0 +1,48 @@
+<domain type='xen'>
+ <name>XenGuest2</name>
+ <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>592896</memory>
+ <currentMemory unit='KiB'>403456</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc' adjustment='reset'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>restart</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+ <disk type='block' device='disk'>
+ <driver name='phy' type='raw'/>
+ <source dev='/dev/HostVG/XenGuest2'/>
+ <target dev='hda' bus='ide'/>
+ </disk>
+ <disk type='file' device='cdrom'>
+ <driver name='qemu' type='raw'/>
+ <source file='/root/boot.iso'/>
+ <target dev='hdc' bus='ide'/>
+ <readonly/>
+ </disk>
+ <interface type='bridge'>
+ <mac address='00:16:3e:66:92:9c'/>
+ <source bridge='xenbr1'/>
+ <script path='vif-bridge'/>
+ <model type='e1000'/>
+ </interface>
+ <input type='mouse' bus='usb'/>
+ <input type='tablet' bus='usb'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'
listen='127.0.0.1' passwd='123poi'>
+ <listen type='address' address='127.0.0.1'/>
+ </graphics>
+ </devices>
+</domain>
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 8c4c82c..6d4aa6d 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -215,6 +215,7 @@ mymain(void)
DO_TEST("new-disk", 3);
DO_TEST("spice", 3);
+ DO_TEST("fullvirt-multiusb", 3);
virObjectUnref(caps);
virObjectUnref(xmlopt);
8:36 a.m.
New subject: [libvirt] [PATCH 4/9] conf: add virDomainHasNet
virDomainNetFindIdx no longer returns info whether device was not found,
or there was multiple matches. Additionally it already handle error
reporting. Introduce virDomainHasNet which does a simple task, without
implicit error reporting.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/conf/domain_conf.c | 21 +++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 23 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9b7ae3f..c430e03 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11908,6 +11908,27 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net)
return matchidx;
}
+bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net)
+{
+ size_t i;
+ bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&net->info,
+
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
+
+ for (i = 0; i < def->nnets; i++) {
+ if (virMacAddrCmp(&def->nets[i]->mac, &net->mac))
+ continue;
+
+ if (PCIAddrSpecified) {
+ if (virDevicePCIAddressEqual(&def->nets[i]->info.addr.pci,
+ &net->info.addr.pci)) {
+ return true;
+ }
+ } else {
+ return true;
+ }
+ }
+ return false;
+}
void
virDomainNetRemoveHostdev(virDomainDefPtr def,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 02ddd93..36c8131 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2601,6 +2601,7 @@ bool virDomainHasDiskMirror(virDomainObjPtr vm);
int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
+bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net);
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetRemove(virDomainDefPtr def, size_t i);
void virDomainNetRemoveHostdev(virDomainDefPtr def, virDomainNetDefPtr net);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba05cc6..5fe6ae0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -292,6 +292,7 @@ virDomainGraphicsTypeToString;
virDomainGraphicsVNCSharePolicyTypeFromString;
virDomainGraphicsVNCSharePolicyTypeToString;
virDomainHasDiskMirror;
+virDomainHasNet;
virDomainHostdevCapsTypeToString;
virDomainHostdevDefAlloc;
virDomainHostdevDefClear;
--
2.1.0
12:53 p.m.
New subject: [libvirt] [PATCH 4/9] conf: add virDomainHasNet
Marek Marczykowski-Górecki wrote:
virDomainNetFindIdx no longer returns info whether device was not
found,
or there was multiple matches. Additionally it already handle error
reporting. Introduce virDomainHasNet which does a simple task, without
implicit error reporting.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/conf/domain_conf.c | 21 +++++++++++++++++++++
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 23 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9b7ae3f..c430e03 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11908,6 +11908,27 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr
net)
return matchidx;
}
+bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net)
Nit: libvirt style is to have return type on separate line.
+{
+ size_t i;
+ bool PCIAddrSpecified = virDomainDeviceAddressIsValid(&net->info,
+
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
+
+ for (i = 0; i < def->nnets; i++) {
+ if (virMacAddrCmp(&def->nets[i]->mac, &net->mac))
+ continue;
+
+ if (PCIAddrSpecified) {
Braces here are fine, where the single statement spans multiple lines.
+ if
(virDevicePCIAddressEqual(&def->nets[i]->info.addr.pci,
+ &net->info.addr.pci)) {
+ return true;
+ }
But not needed here.
ACK. Will fix the nits before pushing.
Regards,
Jim
+ } else {
+ return true;
+ }
+ }
+ return false;
+}
void
virDomainNetRemoveHostdev(virDomainDefPtr def,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 02ddd93..36c8131 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2601,6 +2601,7 @@ bool virDomainHasDiskMirror(virDomainObjPtr vm);
int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
+bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net);
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetRemove(virDomainDefPtr def, size_t i);
void virDomainNetRemoveHostdev(virDomainDefPtr def, virDomainNetDefPtr net);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba05cc6..5fe6ae0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -292,6 +292,7 @@ virDomainGraphicsTypeToString;
virDomainGraphicsVNCSharePolicyTypeFromString;
virDomainGraphicsVNCSharePolicyTypeToString;
virDomainHasDiskMirror;
+virDomainHasNet;
virDomainHostdevCapsTypeToString;
virDomainHostdevDefAlloc;
virDomainHostdevDefClear;
8:36 a.m.
New subject: [libvirt] [PATCH 5/9] libxl: prevent attaching multiple netdevs with the same MAC
It will not be possible to detach such device later. Also improve
logging in such cases.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
Changes in v4:
- use virDomainHasNet instead of virDomainNetFindIdx
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ce3a99b..1313d2e 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
int actualType;
libxl_device_nic nic;
int ret = -1;
+ char mac[VIR_MAC_STRING_BUFLEN];
/* preallocate new slot for device */
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
@@ -2801,6 +2802,13 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
actualType = virDomainNetGetActualType(net);
+ if (virDomainHasNet(vm->def, net)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("device matching mac address %s already exists"),
+ virMacAddrFormat(&net->mac, mac));
+ return -1;
+ }
+
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* This is really a "smart hostdev", so it should be attached
* as a hostdev (the hostdev code will reach over into the
@@ -2879,6 +2887,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
virDomainHostdevDefPtr hostdev;
virDomainHostdevDefPtr found;
virDomainHostdevSubsysPCIPtr pcisrc;
+ char mac[VIR_MAC_STRING_BUFLEN];
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
@@ -2896,6 +2905,12 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
case VIR_DOMAIN_DEVICE_NET:
net = dev->data.net;
+ if (virDomainHasNet(vmdef, net)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("network device with mac %s already
exists."),
+ virMacAddrFormat(&net->mac, mac));
+ return -1;
+ }
if (virDomainNetInsert(vmdef, net))
return -1;
dev->data.net = NULL;
--
2.1.0
12:59 p.m.
New subject: [libvirt] [PATCH 5/9] libxl: prevent attaching multiple netdevs with the same MAC
Marek Marczykowski-Górecki wrote:
It will not be possible to detach such device later. Also improve
logging in such cases.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
Changes in v4:
- use virDomainHasNet instead of virDomainNetFindIdx
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ce3a99b..1313d2e 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
int actualType;
libxl_device_nic nic;
int ret = -1;
+ char mac[VIR_MAC_STRING_BUFLEN];
/* preallocate new slot for device */
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
@@ -2801,6 +2802,13 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
actualType = virDomainNetGetActualType(net);
+ if (virDomainHasNet(vm->def, net)) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("device matching mac address %s already exists"),
+ virMacAddrFormat(&net->mac, mac));
I think the error should be VIR_ERR_INVALID_ARG, like you've done in the
last hunk below. I'll change that, and use the same text in both error
reports, before pushing. Otherwise, ACK.
Regards,
Jim
+ return -1;
+ }
+
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* This is really a "smart hostdev", so it should be attached
* as a hostdev (the hostdev code will reach over into the
@@ -2879,6 +2887,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
virDomainHostdevDefPtr hostdev;
virDomainHostdevDefPtr found;
virDomainHostdevSubsysPCIPtr pcisrc;
+ char mac[VIR_MAC_STRING_BUFLEN];
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
@@ -2896,6 +2905,12 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
case VIR_DOMAIN_DEVICE_NET:
net = dev->data.net;
+ if (virDomainHasNet(vmdef, net)) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("network device with mac %s already
exists."),
+ virMacAddrFormat(&net->mac, mac));
+ return -1;
+ }
if (virDomainNetInsert(vmdef, net))
return -1;
dev->data.net = NULL;
1:16 p.m.
New subject: [libvirt] [PATCH 5/9] libxl: prevent attaching multiple netdevs with the same MAC
Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> It will not be possible to detach such device later. Also improve
> logging in such cases.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> src/libxl/libxl_driver.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> Changes in v4:
> - use virDomainHasNet instead of virDomainNetFindIdx
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index ce3a99b..1313d2e 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2787,6 +2787,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
> int actualType;
> libxl_device_nic nic;
> int ret = -1;
> + char mac[VIR_MAC_STRING_BUFLEN];
>
> /* preallocate new slot for device */
> if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
> @@ -2801,6 +2802,13 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
>
> actualType = virDomainNetGetActualType(net);
>
> + if (virDomainHasNet(vm->def, net)) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("device matching mac address %s already exists"),
>
> + virMacAddrFormat(&net->mac, mac));
>
I think the error should be VIR_ERR_INVALID_ARG, like you've done in the
last hunk below. I'll change that, and use the same text in both error
reports, before pushing. Otherwise, ACK.
With the exception of 4 and 5, the patches in this series are
unrelated. I have a question about 6, but in the meantime it seemed
like a good point to push the first 5. Done now.
Regards,
Jim
8:36 a.m.
New subject: [libvirt] [PATCH 6/9] libxl: support domain config modification in virDomainRestoreFlags
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 1313d2e..d7f5dac 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1456,11 +1456,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
#endif
virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
- if (dxml) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("xml modification unsupported"));
- return -1;
- }
fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr);
if (fd < 0)
@@ -1469,6 +1464,18 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
goto cleanup_unlock;
+ if (dxml) {
+ virDomainDefPtr def2 = NULL;
+
+ if (!(def2 = virDomainDefParseString(dxml, cfg->caps, driver->xmlopt,
+ 1 << VIR_DOMAIN_VIRT_XEN,
+ VIR_DOMAIN_XML_INACTIVE))) {
+ goto cleanup;
+ }
+ virDomainDefFree(def);
+ def = def2;
+ }
+
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
--
2.1.0
1:24 p.m.
New subject: [libvirt] [PATCH 6/9] libxl: support domain config modification in virDomainRestoreFlags
Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 1313d2e..d7f5dac 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1456,11 +1456,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
#endif
virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
- if (dxml) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("xml modification unsupported"));
- return -1;
- }
fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr);
if (fd < 0)
@@ -1469,6 +1464,18 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
goto cleanup_unlock;
+ if (dxml) {
+ virDomainDefPtr def2 = NULL;
+
+ if (!(def2 = virDomainDefParseString(dxml, cfg->caps, driver->xmlopt,
+ 1 << VIR_DOMAIN_VIRT_XEN,
+ VIR_DOMAIN_XML_INACTIVE))) {
+ goto cleanup;
+ }
+ virDomainDefFree(def);
+ def = def2;
+ }
+
This patch looks good and I'm fine with committing it, even though I do
worry a bit about future bug reports where users have changed the config
in incompatible ways and the domain fails to restore. As before, I'd
like to hear what others have to say. Did the qemu driver experience
similar bug reports before checking ABI stability?
One possible solution is to have virDomainDefCheckMetaABIStability() and
virDomainDefCheckDeviceABIStability() functions. The latter could be
used by callers wanting virtual hardware and device ABI stability only.
Together, they provide virDomainDefCheckABIStability() for callers
wanting "strict" ABI stability.
Regards,
Jim
8:36 a.m.
New subject: [libvirt] [PATCH 7/9] libxl: Stubdom emulator type
Xen have feature of having device model in separate domain (called stub
domain). Add <stubdomain> element to allow selecting such configuration.
Emulator path is still used for qemu running in dom0 (if any). Libxl currently
do not allow to select stubdomain path.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
docs/formatdomain.html.in | 13 +++++++++++++
docs/schemas/domaincommon.rng | 10 ++++++++++
src/conf/domain_conf.c | 14 ++++++++++++++
src/conf/domain_conf.h | 3 ++-
src/libxl/libxl_conf.c | 8 ++++++++
5 files changed, 47 insertions(+), 1 deletion(-)
Changes in v4:
- change <emulator type='stubdom'> to separate <stubdomain> element
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fb0a0d1..054f48f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1670,6 +1670,19 @@
</dd>
</dl>
+ <dl>
+ <dt><code>stubdomain</code></dt>
+ <dd>
+ <span class="since">Since 1.2.13</span>, the optional
<code>stubdomain</code>
+ element contains <code>enabled</code> yes/no/default attribute. If
enabled
+ Emulator will be launched in stub domain (Xen only). Currently there
+ is no way to provide path to that emulator. Note that in most cases
+ this emulator will be in addition to one in dom0.
+ <code>emulator</code> element described above still controls the
path
+ to the dom0 instance.
+ </dd>
+ </dl>
+
<h4><a name="elementsDisks">Hard drives, floppy disks,
CDROMs</a></h4>
<p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4e4fe9f..539db39 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2567,6 +2567,13 @@
<ref name="absFilePath"/>
</element>
</define>
+ <define name="stubdomain">
+ <element name="stubdomain">
+ <attribute name="enabled">
+ <ref name="virYesNo"/>
+ </attribute>
+ </element>
+ </define>
<!--
A graphic description, currently in Xen only 2 types are supported:
- sdl with optional display, xauth and fullscreen
@@ -4007,6 +4014,9 @@
<optional>
<ref name="emulator"/>
</optional>
+ <optional>
+ <ref name="stubdomain"/>
+ </optional>
<zeroOrMore>
<choice>
<ref name="disk"/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c430e03..e327e55 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14107,6 +14107,9 @@ virDomainDefParseXML(xmlDocPtr xml,
}
def->emulator = virXPathString("string(./devices/emulator[1])", ctxt);
+ tmp = virXPathString("string(./devices/stubdomain/@enabled)", ctxt);
+ if (tmp)
+ def->stubdomain = virTristateBoolTypeFromString(tmp);
/* analysis of the disk devices */
if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0)
@@ -16064,6 +16067,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
goto error;
}
+ if (src->stubdomain != dst->stubdomain) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target domain stubdomain '%s' does not match source
'%s'"),
+ virTristateBoolTypeToString(dst->stubdomain),
+ virTristateBoolTypeToString(src->stubdomain));
+ goto error;
+ }
+
if (!virDomainDefFeaturesCheckABIStability(src, dst))
goto error;
@@ -20303,6 +20314,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferEscapeString(buf, "<emulator>%s</emulator>\n",
def->emulator);
+ if (def->stubdomain)
+ virBufferAsprintf(buf, "<stubdomain enabled='%s'/>\n",
+ virTristateBoolTypeToString(def->stubdomain));
for (n = 0; n < def->ndisks; n++)
if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 36c8131..446f4f0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2108,9 +2108,10 @@ struct _virDomainDef {
virDomainOSDef os;
char *emulator;
- /* These three options are of type virTristateSwitch,
+ /* These four options are of type virTristateSwitch,
* except VIR_DOMAIN_FEATURE_CAPABILITIES that is of type
* virDomainCapabilitiesPolicy */
+ int stubdomain;
int features[VIR_DOMAIN_FEATURE_LAST];
int apic_eoi;
int hyperv_features[VIR_DOMAIN_HYPERV_LAST];
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 50ef9d8..8ec3c75 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -727,6 +727,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
b_info->device_model_version = libxlDomainGetEmulatorType(def);
}
+ /* In case of stubdom there will be two qemu instances:
+ * - in stubdom (libxl uses hardcoded path for this one),
+ * - in dom0 as a backend for stubdom (if needed).
+ */
+ if (def->stubdomain)
+ libxl_defbool_set(&b_info->device_model_stubdomain,
+ def->stubdomain == VIR_TRISTATE_BOOL_YES);
+
if (def->nserials) {
if (def->nserials > 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.1.0
3:09 p.m.
New subject: [libvirt] [PATCH 7/9] libxl: Stubdom emulator type
Marek Marczykowski-Górecki wrote:
Xen have feature of having device model in separate domain (called
stub
domain). Add <stubdomain> element to allow selecting such configuration.
Emulator path is still used for qemu running in dom0 (if any). Libxl currently
do not allow to select stubdomain path.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
docs/formatdomain.html.in | 13 +++++++++++++
docs/schemas/domaincommon.rng | 10 ++++++++++
src/conf/domain_conf.c | 14 ++++++++++++++
src/conf/domain_conf.h | 3 ++-
src/libxl/libxl_conf.c | 8 ++++++++
5 files changed, 47 insertions(+), 1 deletion(-)
Changes in v4:
- change <emulator type='stubdom'> to separate <stubdomain> element
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fb0a0d1..054f48f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1670,6 +1670,19 @@
</dd>
</dl>
+ <dl>
+ <dt><code>stubdomain</code></dt>
+ <dd>
+ <span class="since">Since 1.2.13</span>, the optional
<code>stubdomain</code>
+ element contains <code>enabled</code> yes/no/default attribute. If
enabled
+ Emulator will be launched in stub domain (Xen only). Currently there
+ is no way to provide path to that emulator. Note that in most cases
+ this emulator will be in addition to one in dom0.
Why does a stubdomain need an emulator (running in dom0) to serve as a
backend? Wouldn't it be better to give the stubdomain direct access to
the resources it needs? Otherwise, It seems to defeat stubdomain's
purpose of moving the emulator execution out of dom0. At least the Xen
wiki describes this as primary motivation for studomains
http://wiki.xen.org/wiki/Device_Model_Stub_Domains
+ <code>emulator</code> element described above
still controls the path
+ to the dom0 instance.
+ </dd>
+ </dl>
+
I think it would be good to add an example above the description. E.g.
along the lines of Daniel's previous example
<devices>
<emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
<stubdomain enabled="yes|no"/>
</devices>
Regards,
Jim
<h4><a name="elementsDisks">Hard drives,
floppy disks, CDROMs</a></h4>
<p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4e4fe9f..539db39 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2567,6 +2567,13 @@
<ref name="absFilePath"/>
</element>
</define>
+ <define name="stubdomain">
+ <element name="stubdomain">
+ <attribute name="enabled">
+ <ref name="virYesNo"/>
+ </attribute>
+ </element>
+ </define>
<!--
A graphic description, currently in Xen only 2 types are supported:
- sdl with optional display, xauth and fullscreen
@@ -4007,6 +4014,9 @@
<optional>
<ref name="emulator"/>
</optional>
+ <optional>
+ <ref name="stubdomain"/>
+ </optional>
<zeroOrMore>
<choice>
<ref name="disk"/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c430e03..e327e55 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14107,6 +14107,9 @@ virDomainDefParseXML(xmlDocPtr xml,
}
def->emulator = virXPathString("string(./devices/emulator[1])", ctxt);
+ tmp = virXPathString("string(./devices/stubdomain/@enabled)", ctxt);
+ if (tmp)
+ def->stubdomain = virTristateBoolTypeFromString(tmp);
/* analysis of the disk devices */
if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0)
@@ -16064,6 +16067,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
goto error;
}
+ if (src->stubdomain != dst->stubdomain) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target domain stubdomain '%s' does not match source
'%s'"),
+ virTristateBoolTypeToString(dst->stubdomain),
+ virTristateBoolTypeToString(src->stubdomain));
+ goto error;
+ }
+
if (!virDomainDefFeaturesCheckABIStability(src, dst))
goto error;
@@ -20303,6 +20314,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferEscapeString(buf, "<emulator>%s</emulator>\n",
def->emulator);
+ if (def->stubdomain)
+ virBufferAsprintf(buf, "<stubdomain enabled='%s'/>\n",
+ virTristateBoolTypeToString(def->stubdomain));
for (n = 0; n < def->ndisks; n++)
if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 36c8131..446f4f0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2108,9 +2108,10 @@ struct _virDomainDef {
virDomainOSDef os;
char *emulator;
- /* These three options are of type virTristateSwitch,
+ /* These four options are of type virTristateSwitch,
* except VIR_DOMAIN_FEATURE_CAPABILITIES that is of type
* virDomainCapabilitiesPolicy */
+ int stubdomain;
int features[VIR_DOMAIN_FEATURE_LAST];
int apic_eoi;
int hyperv_features[VIR_DOMAIN_HYPERV_LAST];
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 50ef9d8..8ec3c75 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -727,6 +727,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
b_info->device_model_version = libxlDomainGetEmulatorType(def);
}
+ /* In case of stubdom there will be two qemu instances:
+ * - in stubdom (libxl uses hardcoded path for this one),
+ * - in dom0 as a backend for stubdom (if needed).
+ */
+ if (def->stubdomain)
+ libxl_defbool_set(&b_info->device_model_stubdomain,
+ def->stubdomain == VIR_TRISTATE_BOOL_YES);
+
if (def->nserials) {
if (def->nserials > 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
5:35 p.m.
New subject: [libvirt] [PATCH 7/9] libxl: Stubdom emulator type
On Fri, Mar 13, 2015 at 02:09:34PM -0600, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Xen have feature of having device model in separate domain (called stub
> domain). Add <stubdomain> element to allow selecting such configuration.
> Emulator path is still used for qemu running in dom0 (if any). Libxl currently
> do not allow to select stubdomain path.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> docs/formatdomain.html.in | 13 +++++++++++++
> docs/schemas/domaincommon.rng | 10 ++++++++++
> src/conf/domain_conf.c | 14 ++++++++++++++
> src/conf/domain_conf.h | 3 ++-
> src/libxl/libxl_conf.c | 8 ++++++++
> 5 files changed, 47 insertions(+), 1 deletion(-)
>
> Changes in v4:
> - change <emulator type='stubdom'> to separate <stubdomain>
element
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index fb0a0d1..054f48f 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1670,6 +1670,19 @@
> </dd>
> </dl>
>
> + <dl>
> + <dt><code>stubdomain</code></dt>
> + <dd>
> + <span class="since">Since 1.2.13</span>, the optional
<code>stubdomain</code>
> + element contains <code>enabled</code> yes/no/default
attribute. If enabled
> + Emulator will be launched in stub domain (Xen only). Currently there
> + is no way to provide path to that emulator. Note that in most cases
>
> + this emulator will be in addition to one in dom0.
Why does a stubdomain need an emulator (running in dom0) to serve as a
backend? Wouldn't it be better to give the stubdomain direct access to
the resources it needs? Otherwise, It seems to defeat stubdomain's
purpose of moving the emulator execution out of dom0. At least the Xen
wiki describes this as primary motivation for studomains
http://wiki.xen.org/wiki/Device_Model_Stub_Domains
Two reasons:
1. To host video/keyboard (vfb/vkb) backends for stubdomain - stubdomain
have no simple way to expose VNC server to the network.
2. To host a console backend. Xenconsoled supports only one console per
domain, but qemu in stubdomain needs at least three (logs, pipe to save
the state and another one to restore the state during save/restore).
Actually in Qubes OS we have our own GUI agent, so the first point is not
needed. And we broke the second one just to get rid of qemu in dom0...
I'll be more than happy if/when xenconsoled will get support for more
than one console device per domain.
> + <code>emulator</code> element described
above still controls the path
> + to the dom0 instance.
> + </dd>
> + </dl>
>
> +
I think it would be good to add an example above the description. E.g.
along the lines of Daniel's previous example
<devices>
<emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
<stubdomain enabled="yes|no"/>
</devices>
Ok, I'll add it in the next patch version (if any).
Regards,
Jim
> <h4><a name="elementsDisks">Hard drives, floppy disks,
CDROMs</a></h4>
>
> <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 4e4fe9f..539db39 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2567,6 +2567,13 @@
> <ref name="absFilePath"/>
> </element>
> </define>
> + <define name="stubdomain">
> + <element name="stubdomain">
> + <attribute name="enabled">
> + <ref name="virYesNo"/>
> + </attribute>
> + </element>
> + </define>
> <!--
> A graphic description, currently in Xen only 2 types are supported:
> - sdl with optional display, xauth and fullscreen
> @@ -4007,6 +4014,9 @@
> <optional>
> <ref name="emulator"/>
> </optional>
> + <optional>
> + <ref name="stubdomain"/>
> + </optional>
> <zeroOrMore>
> <choice>
> <ref name="disk"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c430e03..e327e55 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -14107,6 +14107,9 @@ virDomainDefParseXML(xmlDocPtr xml,
> }
>
> def->emulator = virXPathString("string(./devices/emulator[1])",
ctxt);
> + tmp = virXPathString("string(./devices/stubdomain/@enabled)", ctxt);
> + if (tmp)
> + def->stubdomain = virTristateBoolTypeFromString(tmp);
>
> /* analysis of the disk devices */
> if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) <
0)
> @@ -16064,6 +16067,14 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> goto error;
> }
>
> + if (src->stubdomain != dst->stubdomain) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Target domain stubdomain '%s' does not match source
'%s'"),
> + virTristateBoolTypeToString(dst->stubdomain),
> + virTristateBoolTypeToString(src->stubdomain));
> + goto error;
> + }
> +
> if (!virDomainDefFeaturesCheckABIStability(src, dst))
> goto error;
>
> @@ -20303,6 +20314,9 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>
> virBufferEscapeString(buf, "<emulator>%s</emulator>\n",
> def->emulator);
> + if (def->stubdomain)
> + virBufferAsprintf(buf, "<stubdomain
enabled='%s'/>\n",
> + virTristateBoolTypeToString(def->stubdomain));
>
> for (n = 0; n < def->ndisks; n++)
> if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0)
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 36c8131..446f4f0 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2108,9 +2108,10 @@ struct _virDomainDef {
>
> virDomainOSDef os;
> char *emulator;
> - /* These three options are of type virTristateSwitch,
> + /* These four options are of type virTristateSwitch,
> * except VIR_DOMAIN_FEATURE_CAPABILITIES that is of type
> * virDomainCapabilitiesPolicy */
> + int stubdomain;
> int features[VIR_DOMAIN_FEATURE_LAST];
> int apic_eoi;
> int hyperv_features[VIR_DOMAIN_HYPERV_LAST];
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 50ef9d8..8ec3c75 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -727,6 +727,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> b_info->device_model_version = libxlDomainGetEmulatorType(def);
> }
>
> + /* In case of stubdom there will be two qemu instances:
> + * - in stubdom (libxl uses hardcoded path for this one),
> + * - in dom0 as a backend for stubdom (if needed).
> + */
> + if (def->stubdomain)
> + libxl_defbool_set(&b_info->device_model_stubdomain,
> + def->stubdomain == VIR_TRISTATE_BOOL_YES);
> +
> if (def->nserials) {
> if (def->nserials > 1) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
8:36 a.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8ec3c75..d78d2b2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -735,6 +735,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
libxl_defbool_set(&b_info->device_model_stubdomain,
def->stubdomain == VIR_TRISTATE_BOOL_YES);
+ if (def->os.cmdline && def->os.cmdline[0]) {
+ b_info->extra_hvm = virStringSplit(def->os.cmdline, " ", 0);
+ if (b_info->extra_hvm == NULL) {
+ virReportOOMError();
+ return -1;
+ }
+ }
+
if (def->nserials) {
if (def->nserials > 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.1.0
12:18 p.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 8ec3c75..d78d2b2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -735,6 +735,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
libxl_defbool_set(&b_info->device_model_stubdomain,
def->stubdomain == VIR_TRISTATE_BOOL_YES);
+ if (def->os.cmdline && def->os.cmdline[0]) {
+ b_info->extra_hvm = virStringSplit(def->os.cmdline, " ",
0);
What is the difference between b_info->extra_hvm and b_info->cmdline?
How are these two reconciled in libxl? I.e., what if they contained
different values? Chunyan added the latter with xen commit 11dffa235
when implementing hvm direct kernel boot. She also submitted a patch
for libvirt
https://www.redhat.com/archives/libvir-list/2014-September/msg01006.html
I reworked that patch to support the recent breakout of xl and xm config
in src/xenconfig. While doing so, I fixed other <os> related issues and
posted this series
https://www.redhat.com/archives/libvir-list/2015-March/msg01088.html
Patch 5 in that series supports <cmdline> for HVM guests, but only if
LIBXL_HAVE_BUILDINFO_KERNEL is defined (introduced in Xen4.5 IIRC). Do
we need to populate both b_info->extra_hvm and b_info->cmdline?
Regards,
Jim
12:26 p.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
On Fri, 2015-03-20 at 11:18 -0600, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> src/libxl/libxl_conf.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 8ec3c75..d78d2b2 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -735,6 +735,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> libxl_defbool_set(&b_info->device_model_stubdomain,
> def->stubdomain == VIR_TRISTATE_BOOL_YES);
>
> + if (def->os.cmdline && def->os.cmdline[0]) {
> + b_info->extra_hvm = virStringSplit(def->os.cmdline, "
", 0);
>
What is the difference between b_info->extra_hvm and b_info->cmdline?
According to the comment in libxl_types.idl (i.e. I didn't check the
code) extra_hvm is passed to the qemu (for qemu's own consumption),
cmdline is passed to the guest (used by pv and I suppose by hvm with
direct kernel boot).
Ian.
12:29 p.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
On Fri, Mar 20, 2015 at 11:18:36AM -0600, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> src/libxl/libxl_conf.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 8ec3c75..d78d2b2 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -735,6 +735,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> libxl_defbool_set(&b_info->device_model_stubdomain,
> def->stubdomain == VIR_TRISTATE_BOOL_YES);
>
> + if (def->os.cmdline && def->os.cmdline[0]) {
> + b_info->extra_hvm = virStringSplit(def->os.cmdline, "
", 0);
>
What is the difference between b_info->extra_hvm and b_info->cmdline?
How are these two reconciled in libxl? I.e., what if they contained
In short, I don't think you should use that field at all.
b_info->extra_hvm contains the parameters passed to QEMU specifically
for HVM guest. Those parameters are not meant to be kernel command line.
That field is introduced so that user can append arbitrary parameters to
QEMU. I think it's only a stopgap for things we have not implemented in
libxl or things we don't want to put into libxl. In reality I think it
is rarely used.
different values? Chunyan added the latter with xen commit
11dffa235
when implementing hvm direct kernel boot. She also submitted a patch
for libvirt
https://www.redhat.com/archives/libvir-list/2014-September/msg01006.html
I reworked that patch to support the recent breakout of xl and xm config
in src/xenconfig. While doing so, I fixed other <os> related issues and
posted this series
https://www.redhat.com/archives/libvir-list/2015-March/msg01088.html
Patch 5 in that series supports <cmdline> for HVM guests, but only if
LIBXL_HAVE_BUILDINFO_KERNEL is defined (introduced in Xen4.5 IIRC). Do
we need to populate both b_info->extra_hvm and b_info->cmdline?
No. See above.
Wei.
Regards,
Jim
3:11 p.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
Wei Liu wrote:
On Fri, Mar 20, 2015 at 11:18:36AM -0600, Jim Fehlig wrote:
> Marek Marczykowski-Górecki wrote:
>
>> Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
>> ---
>> src/libxl/libxl_conf.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index 8ec3c75..d78d2b2 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -735,6 +735,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>> libxl_defbool_set(&b_info->device_model_stubdomain,
>> def->stubdomain == VIR_TRISTATE_BOOL_YES);
>>
>> + if (def->os.cmdline && def->os.cmdline[0]) {
>> + b_info->extra_hvm = virStringSplit(def->os.cmdline, "
", 0);
>>
>>
> What is the difference between b_info->extra_hvm and b_info->cmdline?
> How are these two reconciled in libxl? I.e., what if they contained
>
In short, I don't think you should use that field at all.
b_info->extra_hvm contains the parameters passed to QEMU specifically
for HVM guest. Those parameters are not meant to be kernel command line.
That field is introduced so that user can append arbitrary parameters to
QEMU. I think it's only a stopgap for things we have not implemented in
libxl or things we don't want to put into libxl. In reality I think it
is rarely used.
Thanks Wei and Ian for the explanation.
Marek, seems we want b_info->cmdline, if the intent is to "pass cmdline
to HVM guests". Can you review (and test in your setup) the "improve xl
config parsing" series I posted?
Regards,
Jim
4:13 p.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
On Fri, Mar 20, 2015 at 02:11:46PM -0600, Jim Fehlig wrote:
Wei Liu wrote:
> On Fri, Mar 20, 2015 at 11:18:36AM -0600, Jim Fehlig wrote:
>
>> Marek Marczykowski-Górecki wrote:
>>
>>> Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
>>> ---
>>> src/libxl/libxl_conf.c | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>>> index 8ec3c75..d78d2b2 100644
>>> --- a/src/libxl/libxl_conf.c
>>> +++ b/src/libxl/libxl_conf.c
>>> @@ -735,6 +735,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>>> libxl_defbool_set(&b_info->device_model_stubdomain,
>>> def->stubdomain ==
VIR_TRISTATE_BOOL_YES);
>>>
>>> + if (def->os.cmdline && def->os.cmdline[0]) {
>>> + b_info->extra_hvm = virStringSplit(def->os.cmdline,
" ", 0);
>>>
>>>
>> What is the difference between b_info->extra_hvm and b_info->cmdline?
>> How are these two reconciled in libxl? I.e., what if they contained
>>
>
> In short, I don't think you should use that field at all.
>
> b_info->extra_hvm contains the parameters passed to QEMU specifically
> for HVM guest. Those parameters are not meant to be kernel command line.
>
> That field is introduced so that user can append arbitrary parameters to
> QEMU. I think it's only a stopgap for things we have not implemented in
> libxl or things we don't want to put into libxl. In reality I think it
>
> is rarely used.
Thanks Wei and Ian for the explanation.
Marek, seems we want b_info->cmdline, if the intent is to "pass cmdline
to HVM guests". Can you review (and test in your setup) the "improve xl
config parsing" series I posted?
I'll definitely do. But above raises a question - how can I set extra
arguments for qemu? In case of qemu in dom0, it's not a problem because
I can create some wrapper script. But in case of qemu in stubdom, the
only way is to set b_info->extra_hvm. Some additional attributes for
<emulator> tag?
If you're curious why I want to do such things, I have a custom patch for
qemu in stubdom, to have a (LWIP based) DHCP server for the VM there.
Just to get rid of as much as possible from backend domain (which BTW is
not dom0). So I need to pass it some configuration, qemu cmdline seems
like the only feasible option.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
5:26 a.m.
New subject: [libvirt] [PATCH 8/9] libxl: pass cmdline to HVM guests
On Fri, 2015-03-20 at 22:13 +0100, Marek Marczykowski-Górecki wrote:
I'll definitely do. But above raises a question - how can I set
extra
arguments for qemu? In case of qemu in dom0, it's not a problem because
I can create some wrapper script. But in case of qemu in stubdom, the
only way is to set b_info->extra_hvm. Some additional attributes for
<emulator> tag?
There are several components, please can you be precise about which you
want to pass something to.
1. qemu process command line in dom0 (used e.g. for pv backends)
2. command line of the stubdom pv kernel itself
3. qemu "process" command line in stubdom
4. kernel command line in dom0
It's possible that libxl doesn't fully expose all 4 of those. I'm not
sure if #2 is actually useful but it seems like we would certainly want
to be able to expose the other 3 if possible.
b_info->cmdline should be #4, I'm pretty sure.
We also have b_info->extra, ->extra_pv and ->extra_hvm, but I'm not
entirely sure how these map onto #1 and #3.
I'm reasonably sure that ->extra maps to _both_ #1 and #3. It would seem
logical enough if ->extra_hvm mapped to #3 and ->extra_pv mapped to #1,
but I don't know if it actually works that way. It would probably be
quickest to confirm empirically.
With that I'll leave the mapping into livirt xml to others ;-)
Ian.
8:36 a.m.
New subject: [libvirt] [PATCH 9/9] tests: add some simple libxl XML->XML tests
Handle features supported only on xen: driver domains, qemu in
stubdomain.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
tests/Makefile.am | 9 +-
tests/domainschematest | 2 +-
tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml | 42 +++++
tests/xlxml2xmldata/xlxml2xml-hvm.xml | 40 +++++
tests/xlxml2xmldata/xlxml2xml-network-bridged.xml | 38 +++++
.../xlxml2xml-network-driver-domain.xml | 39 +++++
tests/xlxml2xmldata/xlxml2xml-network-routed.xml | 39 +++++
tests/xlxml2xmldata/xlxml2xml-pv.xml | 38 +++++
tests/xlxml2xmltest.c | 189 +++++++++++++++++++++
9 files changed, 433 insertions(+), 3 deletions(-)
create mode 100644 tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-hvm.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-network-bridged.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-network-driver-domain.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-network-routed.xml
create mode 100644 tests/xlxml2xmldata/xlxml2xml-pv.xml
create mode 100644 tests/xlxml2xmltest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 938270c..48648b9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -85,6 +85,7 @@ EXTRA_DIST = \
domainsnapshotxml2xmlout \
fchostdata \
interfaceschemadata \
+ xlxml2xmldata \
lxcconf2xmldata \
lxcxml2xmldata \
lxcxml2xmloutdata \
@@ -230,7 +231,7 @@ test_programs += xml2sexprtest sexpr2xmltest \
endif WITH_XEN
if WITH_LIBXL
-test_programs += xlconfigtest
+test_programs += xlconfigtest xlxml2xmltest
endif WITH_LIBXL
if WITH_QEMU
@@ -515,8 +516,12 @@ xlconfigtest_SOURCES = \
xlconfigtest.c testutilsxen.c testutilsxen.h \
testutils.c testutils.h
xlconfigtest_LDADD =$(libxl_LDADDS)
+xlxml2xmltest_SOURCES = \
+ xlxml2xmltest.c testutilsxen.c testutilsxen.h \
+ testutils.c testutils.h
+xlxml2xmltest_LDADD =$(libxl_LDADDS)
else ! WITH_LIBXL
-EXTRA_DIST += xlconfigtest.c
+EXTRA_DIST += xlconfigtest.c xlxml2xmltest.c
endif ! WITH_LIBXL
QEMUMONITORTESTUTILS_SOURCES = \
diff --git a/tests/domainschematest b/tests/domainschematest
index ba90180..18b442b 100755
--- a/tests/domainschematest
+++ b/tests/domainschematest
@@ -8,7 +8,7 @@ DIRS=""
DIRS="$DIRS domainschemadata qemuxml2argvdata sexpr2xmldata"
DIRS="$DIRS xmconfigdata xml2sexprdata qemuxml2xmloutdata"
DIRS="$DIRS lxcxml2xmldata lxcxml2xmloutdata"
-DIRS="$DIRS bhyvexml2argvdata"
+DIRS="$DIRS bhyvexml2argvdata xlxml2xmldata"
SCHEMA="domain.rng"
check_schema "$DIRS" "$SCHEMA"
diff --git a/tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml
b/tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml
new file mode 100644
index 0000000..4ddfd55
--- /dev/null
+++ b/tests/xlxml2xmldata/xlxml2xml-hvm-stubdom.xml
@@ -0,0 +1,42 @@
+<domain type='xen'>
+ <name>testhvm</name>
+ <uuid>a3d3fa04-dc23-4136-9eab-b579d2930817</uuid>
+ <memory unit='KiB'>524288</memory>
+ <currentMemory unit='KiB'>524288</currentMemory>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ <viridian/>
+ </features>
+ <clock offset='variable' adjustment='0'
basis='localtime'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+ <stubdomain enabled='yes'/>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testhvm/root.img'/>
+ <target dev='xvda' bus='xen'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testhvm/private.img'/>
+ <target dev='xvdb' bus='xen'/>
+ </disk>
+ <interface type='network'>
+ <mac address='00:16:3e:5e:6c:09'/>
+ <source network='default'/>
+ </interface>
+ <input type='tablet' bus='usb'/>
+ </devices>
+</domain>
diff --git a/tests/xlxml2xmldata/xlxml2xml-hvm.xml
b/tests/xlxml2xmldata/xlxml2xml-hvm.xml
new file mode 100644
index 0000000..95d9b84
--- /dev/null
+++ b/tests/xlxml2xmldata/xlxml2xml-hvm.xml
@@ -0,0 +1,40 @@
+<domain type='xen'>
+ <name>testhvm</name>
+ <uuid>a3d3fa04-dc23-4136-9eab-b579d2930817</uuid>
+ <memory unit='KiB'>524288</memory>
+ <currentMemory unit='KiB'>524288</currentMemory>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenfv'>hvm</type>
+ <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='cdrom'/>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ <viridian/>
+ </features>
+ <clock offset='variable' adjustment='0'
basis='localtime'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testhvm/root.img'/>
+ <target dev='xvda' bus='xen'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testhvm/private.img'/>
+ <target dev='xvdb' bus='xen'/>
+ </disk>
+ <interface type='network'>
+ <mac address='00:16:3e:5e:6c:09'/>
+ <source network='default'/>
+ </interface>
+ <input type='tablet' bus='usb'/>
+ </devices>
+</domain>
diff --git a/tests/xlxml2xmldata/xlxml2xml-network-bridged.xml
b/tests/xlxml2xmldata/xlxml2xml-network-bridged.xml
new file mode 100644
index 0000000..a6b8f59
--- /dev/null
+++ b/tests/xlxml2xmldata/xlxml2xml-network-bridged.xml
@@ -0,0 +1,38 @@
+<domain type='xen'>
+ <name>testvm</name>
+ <uuid>900e3685-4998-41ad-a2c9-e496a5ed40a4</uuid>
+ <memory unit='KiB'>4096000</memory>
+ <currentMemory unit='KiB'>409600</currentMemory>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenpv'>linux</type>
+ <kernel>/boot/vmlinuz-3.12.37-1</kernel>
+ <initrd>/boot/initramfs-3.12.37-1.img</initrd>
+ <cmdline>root=/dev/xvda ro nomodeset console=hvc0 rd_NO_PLYMOUTH
3</cmdline>
+ </os>
+ <clock offset='utc' adjustment='reset'>
+ <timer name='tsc' mode='native'/>
+ </clock>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testvm/root.img'/>
+ <target dev='xvda' bus='xen'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='file'/>
+ <source dev='/var/lib/libvirt/images/testvm/home.img'/>
+ <target dev='xvdb' bus='xen'/>
+ </disk>
+ <interface type='bridge'>
+ <mac address='00:16:3e:5e:6c:09'/>
+ <source bridge='xenbr0'/>
+ </interface>
+ <console type='pty'>
+ <target type='xen' port='0'/>
+ </console>
+ </devices>
+</domain>
diff --git a/tests/xlxml2xmldata/xlxml2xml-network-driver-domain.xml
b/tests/xlxml2xmldata/xlxml2xml-network-driver-domain.xml
new file mode 100644
index 0000000..ef0e06c
--- /dev/null
+++ b/tests/xlxml2xmldata/xlxml2xml-network-driver-domain.xml
@@ -0,0 +1,39 @@
+<domain type='xen'>
+ <name>testvm</name>
+ <uuid>900e3685-4998-41ad-a2c9-e496a5ed40a4</uuid>
+ <memory unit='KiB'>4096000</memory>
+ <currentMemory unit='KiB'>409600</currentMemory>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenpv'>linux</type>
+ <kernel>/boot/vmlinuz-3.12.37-1</kernel>
+ <initrd>/boot/initramfs-3.12.37-1.img</initrd>
+ <cmdline>root=/dev/xvda ro nomodeset console=hvc0 rd_NO_PLYMOUTH
3</cmdline>
+ </os>
+ <clock offset='utc' adjustment='reset'>
+ <timer name='tsc' mode='native'/>
+ </clock>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testvm/root.img'/>
+ <target dev='xvda' bus='xen'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='file'/>
+ <source dev='/var/lib/libvirt/images/testvm/home.img'/>
+ <target dev='xvdb' bus='xen'/>
+ </disk>
+ <interface type='bridge'>
+ <mac address='00:16:3e:5e:6c:09'/>
+ <source bridge='xenbr0'/>
+ <backenddomain name='netvm'/>
+ </interface>
+ <console type='pty'>
+ <target type='xen' port='0'/>
+ </console>
+ </devices>
+</domain>
diff --git a/tests/xlxml2xmldata/xlxml2xml-network-routed.xml
b/tests/xlxml2xmldata/xlxml2xml-network-routed.xml
new file mode 100644
index 0000000..fac9d8d
--- /dev/null
+++ b/tests/xlxml2xmldata/xlxml2xml-network-routed.xml
@@ -0,0 +1,39 @@
+<domain type='xen'>
+ <name>testvm</name>
+ <uuid>900e3685-4998-41ad-a2c9-e496a5ed40a4</uuid>
+ <memory unit='KiB'>4096000</memory>
+ <currentMemory unit='KiB'>409600</currentMemory>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenpv'>linux</type>
+ <kernel>/boot/vmlinuz-3.12.37-1</kernel>
+ <initrd>/boot/initramfs-3.12.37-1.img</initrd>
+ <cmdline>root=/dev/xvda ro nomodeset console=hvc0 rd_NO_PLYMOUTH
3</cmdline>
+ </os>
+ <clock offset='utc' adjustment='reset'>
+ <timer name='tsc' mode='native'/>
+ </clock>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testvm/root.img'/>
+ <target dev='xvda' bus='xen'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='file'/>
+ <source dev='/var/lib/libvirt/images/testvm/home.img'/>
+ <target dev='xvdb' bus='xen'/>
+ </disk>
+ <interface type='ethernet'>
+ <mac address='00:16:3e:5e:6c:09'/>
+ <ip address='192.168.0.1' family='ipv4'/>
+ <script path='vif-route'/>
+ </interface>
+ <console type='pty'>
+ <target type='xen' port='0'/>
+ </console>
+ </devices>
+</domain>
diff --git a/tests/xlxml2xmldata/xlxml2xml-pv.xml b/tests/xlxml2xmldata/xlxml2xml-pv.xml
new file mode 100644
index 0000000..b937b8b
--- /dev/null
+++ b/tests/xlxml2xmldata/xlxml2xml-pv.xml
@@ -0,0 +1,38 @@
+<domain type='xen'>
+ <name>testvm</name>
+ <uuid>900e3685-4998-41ad-a2c9-e496a5ed40a4</uuid>
+ <memory unit='KiB'>4096000</memory>
+ <currentMemory unit='KiB'>409600</currentMemory>
+ <vcpu placement='static'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='xenpv'>linux</type>
+ <kernel>/boot/vmlinuz-3.12.37-1</kernel>
+ <initrd>/boot/initramfs-3.12.37-1.img</initrd>
+ <cmdline>root=/dev/xvda ro nomodeset console=hvc0 rd_NO_PLYMOUTH
3</cmdline>
+ </os>
+ <clock offset='utc' adjustment='reset'>
+ <timer name='tsc' mode='native'/>
+ </clock>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <disk type='file' device='disk'>
+ <driver name='file'/>
+ <source file='/var/lib/libvirt/images/testvm/root.img'/>
+ <target dev='xvda' bus='xen'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='file'/>
+ <source dev='/var/lib/libvirt/images/testvm/home.img'/>
+ <target dev='xvdb' bus='xen'/>
+ </disk>
+ <interface type='network'>
+ <mac address='00:16:3e:5e:6c:09'/>
+ <source network='default'/>
+ </interface>
+ <console type='pty'>
+ <target type='xen' port='0'/>
+ </console>
+ </devices>
+</domain>
diff --git a/tests/xlxml2xmltest.c b/tests/xlxml2xmltest.c
new file mode 100644
index 0000000..131e43b
--- /dev/null
+++ b/tests/xlxml2xmltest.c
@@ -0,0 +1,189 @@
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <fcntl.h>
+
+#include "testutils.h"
+
+#ifdef WITH_LIBXL
+
+# include "internal.h"
+# include "libxl/libxl_conf.h"
+# include "libxl/libxl_domain.h"
+# include "testutilsxen.h"
+# include "virstring.h"
+
+# define VIR_FROM_THIS VIR_FROM_NONE
+
+static virCapsPtr caps;
+static virDomainXMLOptionPtr xmlopt;
+
+static int
+testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
+{
+ char *inXmlData = NULL;
+ char *outXmlData = NULL;
+ char *actual = NULL;
+ int ret = -1;
+ virDomainDefPtr def = NULL;
+ unsigned int parse_flags = live ? 0 : VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE;
+ if (!live)
+ format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
+
+ if (virtTestLoadFile(inxml, &inXmlData) < 0)
+ goto fail;
+ if (virtTestLoadFile(outxml, &outXmlData) < 0)
+ goto fail;
+
+ if (!(def = virDomainDefParseString(inXmlData, caps, xmlopt,
+ 1 << VIR_DOMAIN_VIRT_XEN, parse_flags)))
+ goto fail;
+
+ if (!virDomainDefCheckABIStability(def, def)) {
+ fprintf(stderr, "ABI stability check failed on %s", inxml);
+ goto fail;
+ }
+
+ if (!(actual = virDomainDefFormat(def, format_flags)))
+ goto fail;
+
+ if (STRNEQ(outXmlData, actual)) {
+ virtTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml);
+ goto fail;
+ }
+
+ ret = 0;
+ fail:
+ VIR_FREE(inXmlData);
+ VIR_FREE(outXmlData);
+ VIR_FREE(actual);
+ virDomainDefFree(def);
+ return ret;
+}
+
+enum {
+ WHEN_INACTIVE = 1,
+ WHEN_ACTIVE = 2,
+ WHEN_EITHER = 3,
+};
+
+struct testInfo {
+ const char *name;
+ bool different;
+ int when;
+};
+
+static int
+testCompareXMLToXMLHelper(const void *data)
+{
+ const struct testInfo *info = data;
+ char *xml_in = NULL;
+ char *xml_out = NULL;
+ char *xml_out_active = NULL;
+ char *xml_out_inactive = NULL;
+ int ret = -1;
+
+ if (virAsprintf(&xml_in, "%s/xlxml2xmldata/xlxml2xml-%s.xml",
+ abs_srcdir, info->name) < 0 ||
+ virAsprintf(&xml_out, "%s/xlxml2xmldata/xlxml2xml-%s-out.xml",
+ abs_srcdir, info->name) < 0 ||
+ virAsprintf(&xml_out_active,
+ "%s/xlxml2xmldata/xlxml2xml-%s-active.xml",
+ abs_srcdir, info->name) < 0 ||
+ virAsprintf(&xml_out_inactive,
+ "%s/xlxml2xmldata/xlxml2xml-%s-inactive.xml",
+ abs_srcdir, info->name) < 0)
+ goto cleanup;
+
+ if ((info->when & WHEN_INACTIVE)) {
+ char *out;
+ if (!info->different)
+ out = xml_in;
+ else if (virFileExists(xml_out_inactive))
+ out = xml_out_inactive;
+ else
+ out = xml_out;
+
+ if (testCompareXMLToXMLFiles(xml_in, out, false) < 0)
+ goto cleanup;
+ }
+
+ if ((info->when & WHEN_ACTIVE)) {
+ char *out;
+ if (!info->different)
+ out = xml_in;
+ else if (virFileExists(xml_out_active))
+ out = xml_out_active;
+ else
+ out = xml_out;
+
+ if (testCompareXMLToXMLFiles(xml_in, out, true) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(xml_in);
+ VIR_FREE(xml_out);
+ VIR_FREE(xml_out_active);
+ VIR_FREE(xml_out_inactive);
+ return ret;
+}
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+ if ((caps = testXLInitCaps()) == NULL)
+ return EXIT_FAILURE;
+
+ if (!(xmlopt = libxlCreateXMLConf()))
+ return EXIT_FAILURE;
+
+# define DO_TEST_FULL(name, is_different, when) \
+ do { \
+ const struct testInfo info = {name, is_different, when}; \
+ if (virtTestRun("LibXL XML-2-XML " name, \
+ testCompareXMLToXMLHelper, &info) < 0) \
+ ret = -1; \
+ } while (0)
+
+# define DO_TEST(name) \
+ DO_TEST_FULL(name, false, WHEN_EITHER)
+
+# define DO_TEST_DIFFERENT(name) \
+ DO_TEST_FULL(name, true, WHEN_EITHER)
+
+ DO_TEST("hvm");
+ DO_TEST("pv");
+ DO_TEST("hvm-stubdom");
+ DO_TEST("network-bridged");
+ DO_TEST("network-routed");
+ DO_TEST("network-driver-domain");
+
+ virObjectUnref(caps);
+ virObjectUnref(xmlopt);
+
+ return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIRT_TEST_MAIN(mymain)
+
+#else
+
+int
+main(void)
+{
+ return EXIT_AM_SKIP;
+}
+
+#endif /* WITH_QEMU */
--
2.1.0
3:56 p.m.
New subject: [libvirt] [PATCH 9/9] tests: add some simple libxl XML->XML tests
Marek Marczykowski-Górecki wrote:
Handle features supported only on xen: driver domains, qemu in
stubdomain.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
[...]
diff --git a/tests/xlxml2xmltest.c b/tests/xlxml2xmltest.c
new file mode 100644
index 0000000..131e43b
--- /dev/null
+++ b/tests/xlxml2xmltest.c
@@ -0,0 +1,189 @@
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <fcntl.h>
+
+#include "testutils.h"
+
+#ifdef WITH_LIBXL
+
+# include "internal.h"
+# include "libxl/libxl_conf.h"
+# include "libxl/libxl_domain.h"
+# include "testutilsxen.h"
+# include "virstring.h"
+
+# define VIR_FROM_THIS VIR_FROM_NONE
+
+static virCapsPtr caps;
+static virDomainXMLOptionPtr xmlopt;
+
+static int
+testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
+{
+ char *inXmlData = NULL;
+ char *outXmlData = NULL;
+ char *actual = NULL;
+ int ret = -1;
+ virDomainDefPtr def = NULL;
+ unsigned int parse_flags = live ? 0 : VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE;
+ if (!live)
+ format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
+
Perhaps I'm being dense, but it is not clear to me what this is testing
wrt libxl or xenconfig code. xml config is read from a file
+ if (virtTestLoadFile(inxml, &inXmlData) < 0)
+ goto fail;
+ if (virtTestLoadFile(outxml, &outXmlData) < 0)
+ goto fail;
+
+ if (!(def = virDomainDefParseString(inXmlData, caps, xmlopt,
+ 1 << VIR_DOMAIN_VIRT_XEN, parse_flags)))
+ goto fail;
+
parsed with virDomainDefParseString (which does invoke the libxl driver
post-parse callbacks)
+ if (!virDomainDefCheckABIStability(def, def)) {
+ fprintf(stderr, "ABI stability check failed on %s", inxml);
+ goto fail;
+ }
virDomainDefCheckABIStability called with identical def
+
+ if (!(actual = virDomainDefFormat(def, format_flags)))
+ goto fail;
converted back to XML
+
+ if (STRNEQ(outXmlData, actual)) {
+ virtTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml);
+ goto fail;
and then compared with xml config read from a file. I suppose the
post-parse callbacks are being tested. IMO, a more useful test would be
domain XML -> libxl_domain_config -> JSON doc, comparing the output JSON
doc to a template doc. Daniel Berrange started work on such a test,
which I toiled on later, but we've yet to find a nice solution that
works across Xen >= 4.2. This work was last discussed in January
https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
Suggestions welcomed!
Regards,
Jim
4:18 p.m.
New subject: [libvirt] [PATCH 9/9] tests: add some simple libxl XML->XML tests
On Fri, Mar 20, 2015 at 02:56:11PM -0600, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Handle features supported only on xen: driver domains, qemu in
> stubdomain.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
>
[...]
> diff --git a/tests/xlxml2xmltest.c b/tests/xlxml2xmltest.c
> new file mode 100644
> index 0000000..131e43b
> --- /dev/null
> +++ b/tests/xlxml2xmltest.c
> @@ -0,0 +1,189 @@
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <sys/types.h>
> +#include <fcntl.h>
> +
> +#include "testutils.h"
> +
> +#ifdef WITH_LIBXL
> +
> +# include "internal.h"
> +# include "libxl/libxl_conf.h"
> +# include "libxl/libxl_domain.h"
> +# include "testutilsxen.h"
> +# include "virstring.h"
> +
> +# define VIR_FROM_THIS VIR_FROM_NONE
> +
> +static virCapsPtr caps;
> +static virDomainXMLOptionPtr xmlopt;
> +
> +static int
> +testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool live)
> +{
> + char *inXmlData = NULL;
> + char *outXmlData = NULL;
> + char *actual = NULL;
> + int ret = -1;
> + virDomainDefPtr def = NULL;
> + unsigned int parse_flags = live ? 0 : VIR_DOMAIN_DEF_PARSE_INACTIVE;
> + unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE;
> + if (!live)
> + format_flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
> +
>
Perhaps I'm being dense, but it is not clear to me what this is testing
wrt libxl or xenconfig code. xml config is read from a file
> + if (virtTestLoadFile(inxml, &inXmlData) < 0)
> + goto fail;
> + if (virtTestLoadFile(outxml, &outXmlData) < 0)
> + goto fail;
> +
> + if (!(def = virDomainDefParseString(inXmlData, caps, xmlopt,
> + 1 << VIR_DOMAIN_VIRT_XEN,
parse_flags)))
> + goto fail;
> +
>
parsed with virDomainDefParseString (which does invoke the libxl driver
post-parse callbacks)
> + if (!virDomainDefCheckABIStability(def, def)) {
> + fprintf(stderr, "ABI stability check failed on %s", inxml);
> + goto fail;
> + }
>
virDomainDefCheckABIStability called with identical def
> +
> + if (!(actual = virDomainDefFormat(def, format_flags)))
> + goto fail;
>
converted back to XML
> +
> + if (STRNEQ(outXmlData, actual)) {
> + virtTestDifferenceFull(stderr, outXmlData, outxml, actual, inxml);
> + goto fail;
>
and then compared with xml config read from a file. I suppose the
post-parse callbacks are being tested. IMO, a more useful test would be
domain XML -> libxl_domain_config -> JSON doc, comparing the output JSON
doc to a template doc. Daniel Berrange started work on such a test,
which I toiled on later, but we've yet to find a nice solution that
works across Xen >= 4.2. This work was last discussed in January
https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
Suggestions welcomed!
I've written this test before finding already existing one for
XML->xenconfig and xenconfig->XML conversion. Initially it was meant to
verify XML parsing/formating code, but handling those with xenconfig
tests seems to be more sensible. So I think you can just drop this
patch.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
3532
days inactive
3549
days old
26 comments
4 participants
participants (4)
-
Ian Campbell -
Jim Fehlig -
Marek Marczykowski-Górecki -
Wei Liu