[libvirt] [PATCH V2 0/3] correct libxl config file convert

Correct libxl config file type=vif handling. --- Changes in v2: * update xen{Parse,Format}ConfigCommon parameter vif_typename to nativeFomat. To reuse XEN_CONFIG_FORMAT_{XM,XL}, first extract XEN_CONFIG_FORMAT_{XM,XL} to xen_common.h Chunyan Liu (3): extract XEN_CONFIG_FORMAT_XM/XL to xen_common.h xenFormatNet: correct `type=netfront' to 'type=vif' to match libxl xlconfigtest: add test case for type=vif in xl format src/libxl/libxl_driver.c | 14 ++++------ src/xen/xen_driver.h | 3 --- src/xenconfig/xen_common.c | 43 ++++++++++++++++++++++-------- src/xenconfig/xen_common.h | 10 +++++-- src/xenconfig/xen_xl.c | 4 +-- src/xenconfig/xen_xm.c | 4 +-- tests/xlconfigdata/test-vif-typename.cfg | 25 ++++++++++++++++++ tests/xlconfigdata/test-vif-typename.xml | 45 ++++++++++++++++++++++++++++++++ tests/xlconfigtest.c | 1 + 9 files changed, 120 insertions(+), 29 deletions(-) create mode 100644 tests/xlconfigdata/test-vif-typename.cfg create mode 100644 tests/xlconfigdata/test-vif-typename.xml -- 2.1.4

Unify XEN_CONFIG_FORMAT_x and LIBXL_CONFIG_FORMAT_x to XEN_CONFIG_FORMAT_x, and move to xen_common.h. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/libxl/libxl_driver.c | 14 +++++--------- src/xen/xen_driver.h | 3 --- src/xenconfig/xen_common.h | 4 ++++ 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 062d6f8..2c19ddb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -70,10 +70,6 @@ VIR_LOG_INIT("libxl.libxl_driver"); #define LIBXL_DOM_REQ_CRASH 3 #define LIBXL_DOM_REQ_HALT 4 -#define LIBXL_CONFIG_FORMAT_XL "xen-xl" -#define LIBXL_CONFIG_FORMAT_XM "xen-xm" -#define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr" - #define LIBXL_NB_TOTAL_CPU_STAT_PARAM 1 #define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities" @@ -2534,14 +2530,14 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0) goto cleanup; - if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup; if (!(def = xenParseXL(conf, cfg->caps, driver->xmlopt))) goto cleanup; - } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup; @@ -2549,7 +2545,7 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, cfg->caps, driver->xmlopt))) goto cleanup; - } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_SEXPR)) { + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_SEXPR)) { /* only support latest xend config format */ if (!(def = xenParseSxprString(nativeConfig, NULL, @@ -2599,10 +2595,10 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto cleanup; - if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { if (!(conf = xenFormatXL(def, conn))) goto cleanup; - } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { if (!(conf = xenFormatXM(conn, def))) goto cleanup; } else { diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 8578324..5015b31 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -67,9 +67,6 @@ int xenRegister (void); # define MIN_XEN_GUEST_SIZE 64 /* 64 megabytes */ -# define XEN_CONFIG_FORMAT_XM "xen-xm" -# define XEN_CONFIG_FORMAT_SEXPR "xen-sxpr" - # define XEND_DOMAINS_DIR "/var/lib/xend/domains" # define XEN_SCHED_SEDF_NPARAM 6 diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 9ddf210..d96063c 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -27,6 +27,10 @@ # include "virconf.h" # include "domain_conf.h" +#define XEN_CONFIG_FORMAT_XL "xen-xl" +#define XEN_CONFIG_FORMAT_XM "xen-xm" +#define XEN_CONFIG_FORMAT_SEXPR "xen-sxpr" + int xenConfigGetString(virConfPtr conf, const char *name, const char **value, -- 2.1.4

On 05/17/2016 10:34 AM, Chunyan Liu wrote:
Unify XEN_CONFIG_FORMAT_x and LIBXL_CONFIG_FORMAT_x to XEN_CONFIG_FORMAT_x, and move to xen_common.h.
Signed-off-by: Chunyan Liu <cyliu@suse.com> LGTM with the minor nitpick fixed, and we're able to consolidate existent abstraction in libxl with xenconfig.
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
--- src/libxl/libxl_driver.c | 14 +++++--------- src/xen/xen_driver.h | 3 --- src/xenconfig/xen_common.h | 4 ++++ 3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 062d6f8..2c19ddb 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -70,10 +70,6 @@ VIR_LOG_INIT("libxl.libxl_driver"); #define LIBXL_DOM_REQ_CRASH 3 #define LIBXL_DOM_REQ_HALT 4
-#define LIBXL_CONFIG_FORMAT_XL "xen-xl" -#define LIBXL_CONFIG_FORMAT_XM "xen-xm" -#define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr" - #define LIBXL_NB_TOTAL_CPU_STAT_PARAM 1
#define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities" @@ -2534,14 +2530,14 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0) goto cleanup;
- if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup; if (!(def = xenParseXL(conf, cfg->caps, driver->xmlopt))) goto cleanup; - } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup;
@@ -2549,7 +2545,7 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, cfg->caps, driver->xmlopt))) goto cleanup; - } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_SEXPR)) { + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_SEXPR)) { /* only support latest xend config format */ if (!(def = xenParseSxprString(nativeConfig, NULL, @@ -2599,10 +2595,10 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto cleanup;
- if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { if (!(conf = xenFormatXL(def, conn))) goto cleanup; - } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { if (!(conf = xenFormatXM(conn, def))) goto cleanup; } else { diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h index 8578324..5015b31 100644 --- a/src/xen/xen_driver.h +++ b/src/xen/xen_driver.h @@ -67,9 +67,6 @@ int xenRegister (void);
# define MIN_XEN_GUEST_SIZE 64 /* 64 megabytes */
-# define XEN_CONFIG_FORMAT_XM "xen-xm" -# define XEN_CONFIG_FORMAT_SEXPR "xen-sxpr" - # define XEND_DOMAINS_DIR "/var/lib/xend/domains"
# define XEN_SCHED_SEDF_NPARAM 6 diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 9ddf210..d96063c 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -27,6 +27,10 @@ # include "virconf.h" # include "domain_conf.h"
+#define XEN_CONFIG_FORMAT_XL "xen-xl" +#define XEN_CONFIG_FORMAT_XM "xen-xm" +#define XEN_CONFIG_FORMAT_SEXPR "xen-sxpr" Code style requires here to be "# define" instead of "#define", otherwise "make syntax-check" fails.
+ int xenConfigGetString(virConfPtr conf, const char *name, const char **value,

According to current xl.cfg docs and xl codes, it uses type=vif instead of type=netfront. Currently after domxml-to-native, libvirt xml model=netfront will be converted to xl type=netfront. This has no problem before, xen codes for a long time just check type=ioemu, if not, set type to _VIF. Since libxl uses parse_nic_config to avoid duplicate codes, it compares 'type=vif' and 'type=ioemu' for valid parameters, others are considered as invalid, thus we have problem with type=netfront in xl config file. #xl create sles12gm-hvm.orig Parsing config from sles12gm-hvm.orig Invalid parameter `type'. Correct the convertion in libvirt, so that it matchs libxl codes and also xl.cfg. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/xenconfig/xen_common.c | 43 ++++++++++++++++++++++++++++++++----------- src/xenconfig/xen_common.h | 6 ++++-- src/xenconfig/xen_xl.c | 4 ++-- src/xenconfig/xen_xm.c | 4 ++-- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index c6aee69..3e57601 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -801,9 +801,8 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def) return -1; } - static int -xenParseVif(virConfPtr conf, virDomainDefPtr def) +xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) { char *script = NULL; virDomainNetDefPtr net = NULL; @@ -942,7 +941,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) VIR_STRDUP(net->model, model) < 0) goto cleanup; - if (!model[0] && type[0] && STREQ(type, "netfront") && + if (!model[0] && type[0] && STREQ(type, vif_typename) && VIR_STRDUP(net->model, "netfront") < 0) goto cleanup; @@ -1046,7 +1045,8 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, - virCapsPtr caps) + virCapsPtr caps, + const char *nativeFormat) { if (xenParseGeneralMeta(conf, def, caps) < 0) return -1; @@ -1066,8 +1066,17 @@ xenParseConfigCommon(virConfPtr conf, if (xenConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0) return -1; - if (xenParseVif(conf, def) < 0) + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { + if (xenParseVif(conf, def, "vif") < 0) + return -1; + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { + if (xenParseVif(conf, def, "netfront") < 0) + return -1; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); return -1; + } if (xenParsePCI(conf, def) < 0) return -1; @@ -1127,7 +1136,8 @@ static int xenFormatNet(virConnectPtr conn, virConfValuePtr list, virDomainNetDefPtr net, - int hvm) + int hvm, + const char *vif_typename) { virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; @@ -1199,7 +1209,7 @@ xenFormatNet(virConnectPtr conn, virBufferAsprintf(&buf, ",model=%s", net->model); } else { if (net->model != NULL && STREQ(net->model, "netfront")) { - virBufferAddLit(&buf, ",type=netfront"); + virBufferAsprintf(&buf, ",type=%s", vif_typename); } else { if (net->model != NULL) virBufferAsprintf(&buf, ",model=%s", net->model); @@ -1749,7 +1759,8 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def) static int xenFormatVif(virConfPtr conf, virConnectPtr conn, - virDomainDefPtr def) + virDomainDefPtr def, + const char *vif_typename) { virConfValuePtr netVal = NULL; size_t i; @@ -1762,7 +1773,7 @@ xenFormatVif(virConfPtr conf, for (i = 0; i < def->nnets; i++) { if (xenFormatNet(conn, netVal, def->nets[i], - hvm) < 0) + hvm, vif_typename) < 0) goto cleanup; } @@ -1788,7 +1799,8 @@ xenFormatVif(virConfPtr conf, int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, - virConnectPtr conn) + virConnectPtr conn, + const char * nativeFormat) { if (xenFormatGeneralMeta(conf, def) < 0) return -1; @@ -1814,8 +1826,17 @@ xenFormatConfigCommon(virConfPtr conf, if (xenFormatVfb(conf, def) < 0) return -1; - if (xenFormatVif(conf, conn, def) < 0) + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { + if (xenFormatVif(conf, conn, def, "vif") < 0) + return -1; + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { + if (xenFormatVif(conf, conn, def, "netfront") < 0) + return -1; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); return -1; + } if (xenFormatPCI(conf, def) < 0) return -1; diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index d96063c..6361aec 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -58,11 +58,13 @@ int xenConfigCopyStringOpt(virConfPtr conf, int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, - virCapsPtr caps); + virCapsPtr caps, + const char *nativeFormat); int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, - virConnectPtr conn); + virConnectPtr conn, + const char *nativeFormat); int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 889dd40..7372f53 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -499,7 +499,7 @@ xenParseXL(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1; - if (xenParseConfigCommon(conf, def, caps) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XL) < 0) goto cleanup; if (xenParseXLOS(conf, def, caps) < 0) @@ -994,7 +994,7 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn) if (!(conf = virConfNew())) goto cleanup; - if (xenFormatConfigCommon(conf, def, conn) < 0) + if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XL) < 0) goto cleanup; if (xenFormatXLOS(conf, def) < 0) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index e09d97e..34d57de 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -443,7 +443,7 @@ xenParseXM(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1; - if (xenParseConfigCommon(conf, def, caps) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XM) < 0) goto cleanup; if (xenParseXMOS(conf, def) < 0) @@ -586,7 +586,7 @@ xenFormatXM(virConnectPtr conn, if (!(conf = virConfNew())) goto cleanup; - if (xenFormatConfigCommon(conf, def, conn) < 0) + if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XM) < 0) goto cleanup; if (xenFormatXMOS(conf, def) < 0) -- 2.1.4

On 05/17/2016 10:34 AM, Chunyan Liu wrote:
According to current xl.cfg docs and xl codes, it uses type=vif instead of type=netfront.
Currently after domxml-to-native, libvirt xml model=netfront will be converted to xl type=netfront. This has no problem before, xen codes for a long time just check type=ioemu, if not, set type to _VIF.
Since libxl uses parse_nic_config to avoid duplicate codes, it compares 'type=vif' and 'type=ioemu' for valid parameters, others are considered as invalid, thus we have problem with type=netfront in xl config file. #xl create sles12gm-hvm.orig Parsing config from sles12gm-hvm.orig Invalid parameter `type'.
Correct the convertion in libvirt, so that it matchs libxl codes and also xl.cfg.
Signed-off-by: Chunyan Liu <cyliu@suse.com> LGTM with the minor nitpicks fixed:
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
--- src/xenconfig/xen_common.c | 43 ++++++++++++++++++++++++++++++++----------- src/xenconfig/xen_common.h | 6 ++++-- src/xenconfig/xen_xl.c | 4 ++-- src/xenconfig/xen_xm.c | 4 ++-- 4 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index c6aee69..3e57601 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -801,9 +801,8 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def) return -1; }
- Spurious newline.
static int -xenParseVif(virConfPtr conf, virDomainDefPtr def) +xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) { char *script = NULL; virDomainNetDefPtr net = NULL; @@ -942,7 +941,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) VIR_STRDUP(net->model, model) < 0) goto cleanup;
- if (!model[0] && type[0] && STREQ(type, "netfront") && + if (!model[0] && type[0] && STREQ(type, vif_typename) && VIR_STRDUP(net->model, "netfront") < 0) goto cleanup;
@@ -1046,7 +1045,8 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, - virCapsPtr caps) + virCapsPtr caps, + const char *nativeFormat) { if (xenParseGeneralMeta(conf, def, caps) < 0) return -1; @@ -1066,8 +1066,17 @@ xenParseConfigCommon(virConfPtr conf, if (xenConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0) return -1;
- if (xenParseVif(conf, def) < 0) + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { + if (xenParseVif(conf, def, "vif") < 0) + return -1; + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { + if (xenParseVif(conf, def, "netfront") < 0) + return -1; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); return -1; + }
if (xenParsePCI(conf, def) < 0) return -1; @@ -1127,7 +1136,8 @@ static int xenFormatNet(virConnectPtr conn, virConfValuePtr list, virDomainNetDefPtr net, - int hvm) + int hvm, + const char *vif_typename) { virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; @@ -1199,7 +1209,7 @@ xenFormatNet(virConnectPtr conn, virBufferAsprintf(&buf, ",model=%s", net->model); } else { if (net->model != NULL && STREQ(net->model, "netfront")) { - virBufferAddLit(&buf, ",type=netfront"); + virBufferAsprintf(&buf, ",type=%s", vif_typename); } else { if (net->model != NULL) virBufferAsprintf(&buf, ",model=%s", net->model); @@ -1749,7 +1759,8 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def) static int xenFormatVif(virConfPtr conf, virConnectPtr conn, - virDomainDefPtr def) + virDomainDefPtr def, + const char *vif_typename) { virConfValuePtr netVal = NULL; size_t i; @@ -1762,7 +1773,7 @@ xenFormatVif(virConfPtr conf,
for (i = 0; i < def->nnets; i++) { if (xenFormatNet(conn, netVal, def->nets[i], - hvm) < 0) + hvm, vif_typename) < 0) goto cleanup; }
@@ -1788,7 +1799,8 @@ xenFormatVif(virConfPtr conf, int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, - virConnectPtr conn) + virConnectPtr conn, + const char * nativeFormat) Hmm, extra space in parameter declaration - should be *nativeFormat.
{ if (xenFormatGeneralMeta(conf, def) < 0) return -1; @@ -1814,8 +1826,17 @@ xenFormatConfigCommon(virConfPtr conf, if (xenFormatVfb(conf, def) < 0) return -1;
- if (xenFormatVif(conf, conn, def) < 0) + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { + if (xenFormatVif(conf, conn, def, "vif") < 0) + return -1; + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { + if (xenFormatVif(conf, conn, def, "netfront") < 0) + return -1; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); return -1; + }
if (xenFormatPCI(conf, def) < 0) return -1; diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index d96063c..6361aec 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -58,11 +58,13 @@ int xenConfigCopyStringOpt(virConfPtr conf,
int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, - virCapsPtr caps); + virCapsPtr caps, + const char *nativeFormat);
int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, - virConnectPtr conn); + virConnectPtr conn, + const char *nativeFormat);
int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 889dd40..7372f53 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -499,7 +499,7 @@ xenParseXL(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1;
- if (xenParseConfigCommon(conf, def, caps) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XL) < 0) goto cleanup;
if (xenParseXLOS(conf, def, caps) < 0) @@ -994,7 +994,7 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn) if (!(conf = virConfNew())) goto cleanup;
- if (xenFormatConfigCommon(conf, def, conn) < 0) + if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XL) < 0) goto cleanup;
if (xenFormatXLOS(conf, def) < 0) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index e09d97e..34d57de 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -443,7 +443,7 @@ xenParseXM(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1;
- if (xenParseConfigCommon(conf, def, caps) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XM) < 0) goto cleanup;
if (xenParseXMOS(conf, def) < 0) @@ -586,7 +586,7 @@ xenFormatXM(virConnectPtr conn, if (!(conf = virConfNew())) goto cleanup;
- if (xenFormatConfigCommon(conf, def, conn) < 0) + if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XM) < 0) goto cleanup;
if (xenFormatXMOS(conf, def) < 0)

Chunyan Liu wrote:
According to current xl.cfg docs and xl codes, it uses type=vif instead of type=netfront.
Currently after domxml-to-native, libvirt xml model=netfront will be converted to xl type=netfront. This has no problem before, xen codes for a long time just check type=ioemu, if not, set type to _VIF.
Since libxl uses parse_nic_config to avoid duplicate codes, it compares 'type=vif' and 'type=ioemu' for valid parameters, others are considered as invalid, thus we have problem with type=netfront in xl config file. #xl create sles12gm-hvm.orig Parsing config from sles12gm-hvm.orig Invalid parameter `type'.
Correct the convertion in libvirt, so that it matchs libxl codes
s/convertion/conversion/ and s/matchs/matches. I fixed the first one, along with the nits noted by Joao, before pushing. But I only now noticed the second one. Regards, Jim
and also xl.cfg.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- src/xenconfig/xen_common.c | 43 ++++++++++++++++++++++++++++++++----------- src/xenconfig/xen_common.h | 6 ++++-- src/xenconfig/xen_xl.c | 4 ++-- src/xenconfig/xen_xm.c | 4 ++-- 4 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index c6aee69..3e57601 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -801,9 +801,8 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def) return -1; }
- static int -xenParseVif(virConfPtr conf, virDomainDefPtr def) +xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) { char *script = NULL; virDomainNetDefPtr net = NULL; @@ -942,7 +941,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) VIR_STRDUP(net->model, model) < 0) goto cleanup;
- if (!model[0] && type[0] && STREQ(type, "netfront") && + if (!model[0] && type[0] && STREQ(type, vif_typename) && VIR_STRDUP(net->model, "netfront") < 0) goto cleanup;
@@ -1046,7 +1045,8 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, - virCapsPtr caps) + virCapsPtr caps, + const char *nativeFormat) { if (xenParseGeneralMeta(conf, def, caps) < 0) return -1; @@ -1066,8 +1066,17 @@ xenParseConfigCommon(virConfPtr conf, if (xenConfigCopyStringOpt(conf, "device_model", &def->emulator) < 0) return -1;
- if (xenParseVif(conf, def) < 0) + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { + if (xenParseVif(conf, def, "vif") < 0) + return -1; + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { + if (xenParseVif(conf, def, "netfront") < 0) + return -1; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); return -1; + }
if (xenParsePCI(conf, def) < 0) return -1; @@ -1127,7 +1136,8 @@ static int xenFormatNet(virConnectPtr conn, virConfValuePtr list, virDomainNetDefPtr net, - int hvm) + int hvm, + const char *vif_typename) { virBuffer buf = VIR_BUFFER_INITIALIZER; virConfValuePtr val, tmp; @@ -1199,7 +1209,7 @@ xenFormatNet(virConnectPtr conn, virBufferAsprintf(&buf, ",model=%s", net->model); } else { if (net->model != NULL && STREQ(net->model, "netfront")) { - virBufferAddLit(&buf, ",type=netfront"); + virBufferAsprintf(&buf, ",type=%s", vif_typename); } else { if (net->model != NULL) virBufferAsprintf(&buf, ",model=%s", net->model); @@ -1749,7 +1759,8 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def) static int xenFormatVif(virConfPtr conf, virConnectPtr conn, - virDomainDefPtr def) + virDomainDefPtr def, + const char *vif_typename) { virConfValuePtr netVal = NULL; size_t i; @@ -1762,7 +1773,7 @@ xenFormatVif(virConfPtr conf,
for (i = 0; i < def->nnets; i++) { if (xenFormatNet(conn, netVal, def->nets[i], - hvm) < 0) + hvm, vif_typename) < 0) goto cleanup; }
@@ -1788,7 +1799,8 @@ xenFormatVif(virConfPtr conf, int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, - virConnectPtr conn) + virConnectPtr conn, + const char * nativeFormat) { if (xenFormatGeneralMeta(conf, def) < 0) return -1; @@ -1814,8 +1826,17 @@ xenFormatConfigCommon(virConfPtr conf, if (xenFormatVfb(conf, def) < 0) return -1;
- if (xenFormatVif(conf, conn, def) < 0) + if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XL)) { + if (xenFormatVif(conf, conn, def, "vif") < 0) + return -1; + } else if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) { + if (xenFormatVif(conf, conn, def, "netfront") < 0) + return -1; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); return -1; + }
if (xenFormatPCI(conf, def) < 0) return -1; diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index d96063c..6361aec 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -58,11 +58,13 @@ int xenConfigCopyStringOpt(virConfPtr conf,
int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, - virCapsPtr caps); + virCapsPtr caps, + const char *nativeFormat);
int xenFormatConfigCommon(virConfPtr conf, virDomainDefPtr def, - virConnectPtr conn); + virConnectPtr conn, + const char *nativeFormat);
int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def); diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 889dd40..7372f53 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -499,7 +499,7 @@ xenParseXL(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1;
- if (xenParseConfigCommon(conf, def, caps) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XL) < 0) goto cleanup;
if (xenParseXLOS(conf, def, caps) < 0) @@ -994,7 +994,7 @@ xenFormatXL(virDomainDefPtr def, virConnectPtr conn) if (!(conf = virConfNew())) goto cleanup;
- if (xenFormatConfigCommon(conf, def, conn) < 0) + if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XL) < 0) goto cleanup;
if (xenFormatXLOS(conf, def) < 0) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index e09d97e..34d57de 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -443,7 +443,7 @@ xenParseXM(virConfPtr conf, def->virtType = VIR_DOMAIN_VIRT_XEN; def->id = -1;
- if (xenParseConfigCommon(conf, def, caps) < 0) + if (xenParseConfigCommon(conf, def, caps, XEN_CONFIG_FORMAT_XM) < 0) goto cleanup;
if (xenParseXMOS(conf, def) < 0) @@ -586,7 +586,7 @@ xenFormatXM(virConnectPtr conn, if (!(conf = virConfNew())) goto cleanup;
- if (xenFormatConfigCommon(conf, def, conn) < 0) + if (xenFormatConfigCommon(conf, def, conn, XEN_CONFIG_FORMAT_XM) < 0) goto cleanup;
if (xenFormatXMOS(conf, def) < 0)

Signed-off-by: Chunyan Liu <cyliu@suse.com> --- tests/xlconfigdata/test-vif-typename.cfg | 25 ++++++++++++++++++ tests/xlconfigdata/test-vif-typename.xml | 45 ++++++++++++++++++++++++++++++++ tests/xlconfigtest.c | 1 + 3 files changed, 71 insertions(+) create mode 100644 tests/xlconfigdata/test-vif-typename.cfg create mode 100644 tests/xlconfigdata/test-vif-typename.xml diff --git a/tests/xlconfigdata/test-vif-typename.cfg b/tests/xlconfigdata/test-vif-typename.cfg new file mode 100644 index 0000000..2e30777 --- /dev/null +++ b/tests/xlconfigdata/test-vif-typename.cfg @@ -0,0 +1,25 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 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" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,type=vif" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-vif-typename.xml b/tests/xlconfigdata/test-vif-typename.xml new file mode 100644 index 0000000..76e98e3 --- /dev/null +++ b/tests/xlconfigdata/test-vif-typename.xml @@ -0,0 +1,45 @@ +<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='variable' adjustment='0' basis='utc'/> + <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'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='netfront'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='4096' heads='1' primary='yes'/> + </video> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index ba1bed0..6819bad 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -228,6 +228,7 @@ mymain(void) DO_TEST_FORMAT("fullvirt-direct-kernel-boot-extra"); DO_TEST_FORMAT("fullvirt-direct-kernel-boot-bogus-extra"); #endif + DO_TEST("vif-typename"); virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.1.4

On 05/17/2016 10:34 AM, Chunyan Liu wrote:
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- tests/xlconfigdata/test-vif-typename.cfg | 25 ++++++++++++++++++ tests/xlconfigdata/test-vif-typename.xml | 45 ++++++++++++++++++++++++++++++++ tests/xlconfigtest.c | 1 + 3 files changed, 71 insertions(+) create mode 100644 tests/xlconfigdata/test-vif-typename.cfg create mode 100644 tests/xlconfigdata/test-vif-typename.xml
diff --git a/tests/xlconfigdata/test-vif-typename.cfg b/tests/xlconfigdata/test-vif-typename.cfg new file mode 100644 index 0000000..2e30777 --- /dev/null +++ b/tests/xlconfigdata/test-vif-typename.cfg @@ -0,0 +1,25 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-dm" Commit b90c4b5f5 changed test data files to use qemu-system-i386 as opposed to qemu traditional, so probably this needs to be inline with that change.
+sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,type=vif" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-vif-typename.xml b/tests/xlconfigdata/test-vif-typename.xml new file mode 100644 index 0000000..76e98e3 --- /dev/null +++ b/tests/xlconfigdata/test-vif-typename.xml @@ -0,0 +1,45 @@ +<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='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-dm</emulator> Same here.
+ <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <script path='vif-bridge'/> + <model type='netfront'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='4096' heads='1' primary='yes'/> And probably vram needs to be adjusted to 8192 - reflecting the change above mentioned.
+ </video> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index ba1bed0..6819bad 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -228,6 +228,7 @@ mymain(void) DO_TEST_FORMAT("fullvirt-direct-kernel-boot-extra"); DO_TEST_FORMAT("fullvirt-direct-kernel-boot-bogus-extra"); #endif + DO_TEST("vif-typename");
virObjectUnref(caps); virObjectUnref(xmlopt);

Chunyan Liu wrote:
Correct libxl config file type=vif handling.
--- Changes in v2: * update xen{Parse,Format}ConfigCommon parameter vif_typename to nativeFomat. To reuse XEN_CONFIG_FORMAT_{XM,XL}, first extract XEN_CONFIG_FORMAT_{XM,XL} to xen_common.h
Chunyan Liu (3): extract XEN_CONFIG_FORMAT_XM/XL to xen_common.h xenFormatNet: correct `type=netfront' to 'type=vif' to match libxl xlconfigtest: add test case for type=vif in xl format
src/libxl/libxl_driver.c | 14 ++++------ src/xen/xen_driver.h | 3 --- src/xenconfig/xen_common.c | 43 ++++++++++++++++++++++-------- src/xenconfig/xen_common.h | 10 +++++-- src/xenconfig/xen_xl.c | 4 +-- src/xenconfig/xen_xm.c | 4 +-- tests/xlconfigdata/test-vif-typename.cfg | 25 ++++++++++++++++++ tests/xlconfigdata/test-vif-typename.xml | 45 ++++++++++++++++++++++++++++++++ tests/xlconfigtest.c | 1 + 9 files changed, 120 insertions(+), 29 deletions(-) create mode 100644 tests/xlconfigdata/test-vif-typename.cfg create mode 100644 tests/xlconfigdata/test-vif-typename.xml
ACK series. I've fixed the nits noted by Joao and pushed it. Regards, Jim
participants (3)
-
Chunyan Liu
-
Jim Fehlig
-
Joao Martins