[libvirt] [PATCH RESENT 07/12] conf: support backend domain name in disk and network devices
Jim Fehlig
jfehlig at suse.com
Wed May 22 15:22:32 UTC 2013
Marek Marczykowski wrote:
> At least Xen supports backend drivers in another domain (aka "driver
> domain"). This patch introduces XML config option for such setting as
> 'domain' element with 'name' attribute. Verification its content is left
> for the driver.
>
> In the future some option will be needed for USB devices (hostdev
> objects), but for now libxl doesn't have support for PVUSB.
> ---
> docs/schemas/domaincommon.rng | 14 ++++++++++++++
> src/conf/domain_conf.c | 27 +++++++++++++++++++++++++++
> src/conf/domain_conf.h | 2 ++
> 3 files changed, 43 insertions(+)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8d7e6db..1423187 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -869,6 +869,13 @@
> </optional>
> <ref name="target"/>
> <optional>
> + <element name="domain">
> + <attribute name="name">
> + <ref name="domainName"/>
> + </attribute>
> + </element>
> + </optional>
> + <optional>
> <ref name="deviceBoot"/>
> </optional>
> <optional>
> @@ -1834,6 +1841,13 @@
> </element>
> </optional>
> <optional>
> + <element name="domain">
> + <attribute name="name">
> + <ref name="domainName"/>
> + </attribute>
> + </element>
> + </optional>
> + <optional>
>
I'm certainly no expert in RNG, but do these need to include <empty/>?
BTW, you'll also need to document this in docs/formatdomain.html.in.
> <element name="model">
> <attribute name="type">
> <data type="string">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 257a265..bf1fec6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1103,6 +1103,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
> VIR_FREE(def->vendor);
> VIR_FREE(def->product);
> VIR_FREE(def->script);
> + VIR_FREE(def->domain_name);
> if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
> VIR_FREE(def->auth.secret.usage);
> virStorageEncryptionFree(def->encryption);
> @@ -1228,6 +1229,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
>
> VIR_FREE(def->virtPortProfile);
> VIR_FREE(def->script);
> + VIR_FREE(def->domain_name);
> VIR_FREE(def->ifname);
>
> virDomainDeviceInfoClear(&def->info);
> @@ -3995,6 +3997,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> char *vendor = NULL;
> char *product = NULL;
> char *script = NULL;
> + char *domain_name = NULL;
> int expected_secret_usage = -1;
> int auth_secret_usage = -1;
>
> @@ -4153,6 +4156,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> } else if (!script &&
> xmlStrEqual(cur->name, BAD_CAST "script")) {
> script = virXMLPropString(cur, "path");
> + } else if (!domain_name &&
> + xmlStrEqual(cur->name, BAD_CAST "domain")) {
> + domain_name = virXMLPropString(cur, "name");
> } else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) {
> if (virXPathUInt("string(./geometry/@cyls)",
> ctxt, &def->geometry.cylinders) < 0) {
> @@ -4447,6 +4453,11 @@ virDomainDiskDefParseXML(virCapsPtr caps,
> ctxt->node = saved_node;
> }
>
> + if (domain_name != NULL) {
> + def->domain_name = domain_name;
> + domain_name = NULL;
> + }
> +
>
Is the 'if' necessary here? It looks like the other fields of def are
unconditionally assigned, e.g.
def->src = source;
source = NULL;
> if (target == NULL) {
> virReportError(VIR_ERR_NO_TARGET,
> source ? "%s" : NULL, source);
> @@ -4796,6 +4807,7 @@ cleanup:
> VIR_FREE(vendor);
> VIR_FREE(product);
> VIR_FREE(script);
> + VIR_FREE(domain_name);
>
> ctxt->node = save_ctxt;
> return def;
> @@ -5353,6 +5365,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
> char *mode = NULL;
> char *linkstate = NULL;
> char *addrtype = NULL;
> + char *domain_name = NULL;
> virNWFilterHashTablePtr filterparams = NULL;
> virDomainActualNetDefPtr actual = NULL;
> xmlNodePtr oldnode = ctxt->node;
> @@ -5451,6 +5464,9 @@ virDomainNetDefParseXML(virCapsPtr caps,
> } else if (!script &&
> xmlStrEqual(cur->name, BAD_CAST "script")) {
> script = virXMLPropString(cur, "path");
> + } else if (!domain_name &&
> + xmlStrEqual(cur->name, BAD_CAST "domain")) {
> + domain_name = virXMLPropString(cur, "name");
> } else if (xmlStrEqual(cur->name, BAD_CAST "model")) {
> model = virXMLPropString(cur, "type");
> } else if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
> @@ -5682,6 +5698,10 @@ virDomainNetDefParseXML(virCapsPtr caps,
> def->script = script;
> script = NULL;
> }
> + if (domain_name != NULL) {
> + def->domain_name = domain_name;
> + domain_name = NULL;
> + }
>
The pattern in this function is to conditionally assign def->foo, so I
guess my above comment is a nit.
> if (ifname != NULL) {
> def->ifname = ifname;
> ifname = NULL;
> @@ -5808,6 +5828,7 @@ cleanup:
> VIR_FREE(mode);
> VIR_FREE(linkstate);
> VIR_FREE(addrtype);
> + VIR_FREE(domain_name);
> virNWFilterHashTableFree(filterparams);
>
> return def;
> @@ -12909,6 +12930,10 @@ virDomainDiskDefFormat(virBufferPtr buf,
>
> virBufferEscapeString(buf, " <script path='%s'/>\n", def->script);
>
> + if (def->domain_name) {
> + virBufferEscapeString(buf, " <domain name='%s'/>\n", def->domain_name);
> + }
> +
> virDomainDiskGeometryDefFormat(buf, def);
> virDomainDiskBlockIoDefFormat(buf, def);
>
> @@ -13456,6 +13481,8 @@ virDomainNetDefFormat(virBufferPtr buf,
> return -1;
> virBufferEscapeString(buf, "<script path='%s'/>\n",
> def->script);
> + if (def->domain_name)
> + virBufferEscapeString(buf, "<domain name='%s'/>\n", def->domain_name);
> if (def->ifname &&
> !((flags & VIR_DOMAIN_XML_INACTIVE) &&
> (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index d55d209..db3002b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -669,6 +669,7 @@ struct _virDomainDiskDef {
> int rawio; /* no = 0, yes = 1 */
> int sgio; /* enum virDomainDiskSGIO */
> char *script;
> + char *domain_name; /* backend domain name */
>
> size_t nseclabels;
> virSecurityDeviceLabelDefPtr *seclabels;
> @@ -919,6 +920,7 @@ struct _virDomainNetDef {
> unsigned long sndbuf;
> } tune;
> char *script;
> + char *domain_name; /* backend domain name */
> char *ifname;
> virDomainDeviceInfo info;
> char *filter;
>
Also, with danpb's "improving unit test coverage" mail fresh in my head,
this needs to include a test? I think changes to domain conf generally
require a one anyhow. Looks good, but a V2 is needed with the missing
documentation.
Regards,
Jim
More information about the libvir-list
mailing list