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

Re: [libvirt] [PATCH 4/5] Add support for qemu's guestfwd



On Wed, Nov 04, 2009 at 04:22:01PM +0000, Matthew Booth wrote:
> This patch allows the following to be specified in a qemu domain:
> 
> <channel type='pipe'>
>   <source path='/tmp/guestfwd'/>
>   <target type='guestfwd' address='10.0.2.1' port='4600'/>
> </channel>
> 
> This will output the following on the qemu command line:
> 
> -chardev pipe,id=channel0,path=/tmp/guestfwd \
> -net user,guestfwd=tcp:10.0.2.1:4600-chardev:channel0
> 
> * docs/schemas/domain.rng: Add <channel> and <guestfwd> elements
> * proxy/Makefile.am: add network.c as dep of domain_conf.c
> * src/conf/domain_conf.[ch]: Add xml parsing/formatting for channel and guestfwd
> * src/qemu/qemu_conf.c: Add argument output for guestfwd
> * tests/qemuxml2(argv|xml)test.c: Add test for guestfwd domain syntax
> ---
>  docs/schemas/domain.rng                            |   89 ++++++----
>  proxy/Makefile.am                                  |    1 +
>  src/conf/domain_conf.c                             |  189 ++++++++++++++++++--
>  src/conf/domain_conf.h                             |    6 +
>  src/qemu/qemu_conf.c                               |   64 +++++++
>  .../qemuxml2argv-channel-guestfwd.args             |    1 +
>  .../qemuxml2argv-channel-guestfwd.xml              |   26 +++
>  tests/qemuxml2argvtest.c                           |    4 +-
>  tests/qemuxml2xmltest.c                            |    1 +
>  9 files changed, 332 insertions(+), 49 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.xml
> 
> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
> index 0a6ab61..b75f17e 100644
> --- a/docs/schemas/domain.rng
> +++ b/docs/schemas/domain.rng
> @@ -930,6 +930,19 @@
>        definition doesn't fully specify the constraints on this node.
>      -->
>    <define name="qemucdev">
> +    <ref name="qemucdevSrcType"/>
> +    <interleave>
> +      <ref name="qemucdevSrcDef"/>
> +      <optional>
> +        <element name="target">
> +          <optional>
> +            <attribute name="port"/>
> +          </optional>
> +        </element>
> +      </optional>
> +    </interleave>
> +  </define>
> +  <define name="qemucdevSrcType">
>      <attribute name="type">
>        <choice>
>          <value>dev</value>
> @@ -944,43 +957,36 @@
>          <value>pty</value>
>        </choice>
>      </attribute>
> -    <interleave>
> -      <optional>
> -        <oneOrMore>
> -          <element name="source">
> -            <optional>
> -              <attribute name="mode"/>
> -            </optional>
> -            <optional>
> -              <attribute name="path"/>
> -            </optional>
> -            <optional>
> -              <attribute name="host"/>
> -            </optional>
> -            <optional>
> -              <attribute name="service"/>
> -            </optional>
> -            <optional>
> -              <attribute name="wiremode"/>
> -            </optional>
> -          </element>
> -        </oneOrMore>
> -      </optional>
> -      <optional>
> -        <element name="protocol">
> +  </define>
> +  <define name="qemucdevSrcDef">
> +    <optional>
> +      <oneOrMore>
> +        <element name="source">
>            <optional>
> -            <attribute name="type"/>
> +            <attribute name="mode"/>
>            </optional>
> -        </element>
> -      </optional>
> -      <optional>
> -        <element name="target">
>            <optional>
> -            <attribute name="port"/>
> +            <attribute name="path"/>
> +          </optional>
> +          <optional>
> +            <attribute name="host"/>
> +          </optional>
> +          <optional>
> +            <attribute name="service"/>
> +          </optional>
> +          <optional>
> +            <attribute name="wiremode"/>
>            </optional>
>          </element>
> -      </optional>
> -    </interleave>
> +      </oneOrMore>
> +    </optional>
> +    <optional>
> +      <element name="protocol">
> +        <optional>
> +          <attribute name="type"/>
> +        </optional>
> +      </element>
> +    </optional>
>    </define>
>    <!--
>        The description for a console
> @@ -1044,6 +1050,24 @@
>        <ref name="qemucdev"/>
>      </element>
>    </define>
> +  <define name="guestfwdTarget">
> +    <element name="target">
> +        <attribute name="type">
> +            <value>guestfwd</value>
> +        </attribute>
> +        <attribute name="address"/>
> +        <attribute name="port"/>
> +    </element>
> +  </define>
> +  <define name="channel">
> +    <element name="channel">
> +      <ref name="qemucdevSrcType"/>
> +      <interleave>
> +        <ref name="qemucdevSrcDef"/>
> +        <ref name="guestfwdTarget"/>
> +      </interleave>
> +    </element>
> +  </define>
>    <define name="input">
>      <element name="input">
>        <attribute name="type">
> @@ -1158,6 +1182,7 @@
>              <ref name="console"/>
>              <ref name="parallel"/>
>              <ref name="serial"/>
> +            <ref name="channel"/>
>            </choice>
>          </zeroOrMore>
>          <optional>

 Okay I had done a review of the XML extension before, I'm fine with
 <channel> :-)


> @@ -1346,18 +1362,15 @@ virDomainChrDefParseXML(virConnectPtr conn,
>      else if ((def->type = virDomainChrTypeFromString(type)) < 0)
>          def->type = VIR_DOMAIN_CHR_TYPE_NULL;
>  
> -    targetType = (const char *) node->name;
> -    if (targetType == NULL) {
> -        /* Shouldn't be possible */
> -        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> -                             "node->name is NULL at %s:%i",
> -                             __FILE__, __LINE__);
> -        return NULL;
> -    }
> -    if ((def->targetType = virDomainChrTargetTypeFromString(targetType)) < 0) {
> -        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> -                             _("unknown target type for character device: %s"),
> -                             targetType);
> +    nodeName = (const char *) node->name;
> +    if ((def->targetType = virDomainChrTargetTypeFromString(nodeName)) < 0) {
> +        /* channel is handled below */
> +        if(STRNEQ(nodeName, "channel")) {
> +            virDomainReportError(conn, VIR_ERR_INVALID_DOMAIN,
> +                              _("unknown target type for character device: %s"),
> +                                 nodeName);
> +            return NULL;
> +        }
>          def->targetType = VIR_DOMAIN_CHR_TARGET_TYPE_NULL;
>      }

  I had to manually apply this since I had modified that part in 1/5 :-)

  Okay, patch looks fine. the rng and conf part could probably have been
isolated as a first patch and then patch 3 and the qemu side could have
been merged as a second patch, but in the end this doesn't change much
:-)

   Pushed, thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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