[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 03/14] conf: support backend domain name in disk and network devices



Marek Marczykowski-Górecki 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.
>
> Changes in v2:
>  - describe in docs/formatdomain.html.in
>  - enforce empty domain tag (only 'name' attribute allowed)
>   

Remove patch change history.

> Signed-off-by: Marek Marczykowski-Górecki <marmarek invisiblethingslab com>
> ---
>  docs/formatdomain.html.in     | 29 +++++++++++++++++++++++++++++
>  docs/schemas/domaincommon.rng | 16 ++++++++++++++++
>  src/conf/domain_conf.c        | 28 ++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        |  2 ++
>  4 files changed, 75 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 755d084..1ca8ae0 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
>   

Some grammar nits below, since this is the website documentation...

> @@ -1751,6 +1751,13 @@
>            </li>
>          </ul>
>        </dd>
> +      <dt><code>domain</code></dt>
> +      <dd>The optional <code>domain</code> element allows specifying backend
>   

s/specifying backend/specifying a backend/

> +        domain (aka driver domain) for the device. If real device/file resides
>   

s/If real/If the real/

> +        in some other domain on the same host, you can use <code>name</code>
>   

s/you can use/use the/

> +        attribute to specify its name.
> +        <span class="since">Since 1.0.7 (Xen only)</span>
> +      </dd>
>        <dt><code>boot</code></dt>
>        <dd>Specifies that the disk is bootable. The <code>order</code>
>          attribute determines the order in which devices will be tried during
> @@ -3554,6 +3561,28 @@ qemu-kvm -net nic,model=? /dev/null
>        element is unspecified is to have the link state <code>up</code>.
>        <span class="since">Since 0.9.5</span>
>      </p>
> +    <h5><a name="elementDomain">Setting up network backend in driver domain</a></h5>
>   

Should read, "Setting up a network backend in a driver domain"

> +<pre>
> +  ...
> +  &lt;devices&gt;
> +    ...
> +    &lt;interface type='bridge'&gt;
> +      &lt;source bridge='br0'/&gt;
> +      <b>&lt;domain name='netvm'/&gt;</b>
> +    &lt;/interface&gt;
> +    ...
> +  &lt;/devices&gt;
> +  ...</pre>
> +
> +    <p>
> +      The optional <code>domain</code> element allows specifying backend
>   

s/specifying backend/specifying a backend/

> +      domain (aka driver domain) for the device. Use <code>name</code> attribute
>   

s/Use/Use the/

> +      to specify its name. You can use it to create direct network link between
>   

s/create direct/create a direct/

> +      domains (so data will not go through host system). Use with type 'ethernet'
> +      to create plain network link, or with 'bridge' to connect to some bridge
> +      inside driver domain.
>   

s/inside driver/inside the driver/

> +      <span class="since">Since 1.0.7 (Xen only)</span>
> +    </p>
>  
>      <h4><a name="elementsInput">Input devices</a></h4>
>  
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 1eb2f68..6fe4d90 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -908,6 +908,14 @@
>        </optional>
>        <ref name="target"/>
>        <optional>
> +        <element name="domain">
> +          <attribute name="name">
> +            <ref name="domainName"/>
> +          </attribute>
> +          <empty/>
> +        </element>
> +      </optional>
> +      <optional>
>          <ref name="deviceBoot"/>
>        </optional>
>        <optional>
> @@ -1981,6 +1989,14 @@
>          </element>
>        </optional>
>        <optional>
> +        <element name="domain">
> +          <attribute name="name">
> +            <ref name="domainName"/>
> +          </attribute>
> +          <empty/>
> +        </element>
> +      </optional>
> +      <optional>
>          <element name="model">
>            <attribute name="type">
>              <data type="string">
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2373397..5350c56 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1183,6 +1183,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
>      VIR_FREE(def->wwn);
>      VIR_FREE(def->vendor);
>      VIR_FREE(def->product);
> +    VIR_FREE(def->domain_name);
>      if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
>          VIR_FREE(def->auth.secret.usage);
>      virStorageEncryptionFree(def->encryption);
> @@ -1308,6 +1309,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);
> @@ -4693,6 +4695,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *vendor = NULL;
>      char *product = NULL;
>      char *discard = NULL;
> +    char *domain_name = NULL;
>      int expected_secret_usage = -1;
>      int auth_secret_usage = -1;
>  
> @@ -4853,6 +4856,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  if (target &&
>                      STRPREFIX(target, "ioemu:"))
>                      memmove(target, target+6, strlen(target)-5);
> +            } 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) {
> @@ -5148,6 +5154,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>          ctxt->node = saved_node;
>      }
>  
> +    if (domain_name != NULL) {
> +        def->domain_name = domain_name;
> +        domain_name = NULL;
> +    }
> +
>      if (target == NULL) {
>          if (def->srcpool) {
>              char *tmp;
> @@ -5504,6 +5515,7 @@ cleanup:
>      VIR_FREE(wwn);
>      VIR_FREE(vendor);
>      VIR_FREE(product);
> +    VIR_FREE(domain_name);
>  
>      ctxt->node = save_ctxt;
>      return def;
> @@ -6109,6 +6121,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *mode = NULL;
>      char *linkstate = NULL;
>      char *addrtype = NULL;
> +    char *domain_name = NULL;
>      virNWFilterHashTablePtr filterparams = NULL;
>      virDomainActualNetDefPtr actual = NULL;
>      xmlNodePtr oldnode = ctxt->node;
> @@ -6207,6 +6220,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>              } 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")) {
> @@ -6437,6 +6453,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>          def->script = script;
>          script = NULL;
>      }
> +    if (domain_name != NULL) {
> +        def->domain_name = domain_name;
> +        domain_name = NULL;
> +    }
>      if (ifname != NULL) {
>          def->ifname = ifname;
>          ifname = NULL;
> @@ -6574,6 +6594,7 @@ cleanup:
>      VIR_FREE(mode);
>      VIR_FREE(linkstate);
>      VIR_FREE(addrtype);
> +    VIR_FREE(domain_name);
>      virNWFilterHashTableFree(filterparams);
>  
>      return def;
> @@ -14018,6 +14039,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
>  
>      if (virDomainDiskSourceDefFormat(buf, def) < 0)
>          return -1;
> +
> +    if (def->domain_name) {
> +        virBufferEscapeString(buf, "      <domain name='%s'/>\n", def->domain_name);
> +    }
> +
>      virDomainDiskGeometryDefFormat(buf, def);
>      virDomainDiskBlockIoDefFormat(buf, def);
>  
> @@ -14601,6 +14627,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 5b159ac..24d3809 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -718,6 +718,7 @@ struct _virDomainDiskDef {
>      int rawio; /* no = 0, yes = 1 */
>      int sgio; /* enum virDomainDeviceSGIO */
>      int discard; /* enum virDomainDiskDiscard */
> +    char *domain_name; /* backend domain name */
>  
>      size_t nseclabels;
>      virSecurityDeviceLabelDefPtr *seclabels;
> @@ -981,6 +982,7 @@ struct _virDomainNetDef {
>          unsigned long sndbuf;
>      } tune;
>      char *script;
> +    char *domain_name; /* backend domain name */
>      char *ifname;
>      virDomainDeviceInfo info;
>      char *filter;
>   

Looks good to me with exception of the few nits, but would be nice to
have another set of eyes review the schema changes.

Regards,
Jim


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]