[libvirt] [PATCHv2 3/7] conf: introduce <vsock> element

[Peter Krempa]

On Tue, May 29, 2018 at 13:36:51 +0200, Ján Tomko wrote:
> On Tue, May 29, 2018 at 10:26:20AM +0200, Peter Krempa wrote:
> > On Thu, May 24, 2018 at 12:39:11 +0200, Ján Tomko wrote:
> > > Add a new 'vsock' element for the vsock device.
> > > The 'model' attribute is optional.
> > > A <source cid> subelement should be used to specify the guest cid,
> > > or <source auto='yes'/> should be used.
> > > 
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1291851
> > > ---
> > >  docs/formatdomain.html.in                     |  20 +++
> > >  docs/schemas/domaincommon.rng                 |  29 ++++
> > >  src/conf/domain_conf.c                        | 195 +++++++++++++++++++++++++-
> > >  src/conf/domain_conf.h                        |  17 +++
> > >  src/qemu/qemu_domain.c                        |   1 +
> > >  src/qemu/qemu_domain_address.c                |  11 ++
> > >  src/qemu/qemu_driver.c                        |   6 +
> > >  src/qemu/qemu_hotplug.c                       |   1 +
> > >  tests/qemuxml2argvdata/vhost-vsock-auto.xml   |  35 +++++
> > >  tests/qemuxml2argvdata/vhost-vsock.xml        |  36 +++++
> > >  tests/qemuxml2xmloutdata/vhost-vsock-auto.xml |  36 +++++
> > >  tests/qemuxml2xmloutdata/vhost-vsock.xml      |   1 +
> > >  tests/qemuxml2xmltest.c                       |   3 +
> > >  13 files changed, 390 insertions(+), 1 deletion(-)
> > >  create mode 100644 tests/qemuxml2argvdata/vhost-vsock-auto.xml
> > >  create mode 100644 tests/qemuxml2argvdata/vhost-vsock.xml
> > >  create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
> > >  create mode 120000 tests/qemuxml2xmloutdata/vhost-vsock.xml
> > 
> 
> > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > > index 71ac3d079c..3ea5c91773 100644
> > > --- a/docs/schemas/domaincommon.rng
> > > +++ b/docs/schemas/domaincommon.rng
> > > @@ -4140,6 +4140,32 @@
> > >      </optional>
> > >    </define>
> > > 
> > > +  <define name="vsock">
> > > +    <element name="vsock">
> > > +      <optional>
> > > +        <attribute name="model">
> > > +          <value>virtio</value>
> > > +        </attribute>
> > > +      </optional>
> > > +      <interleave>
> > > +        <element name="source">
> > 
> > So, source is mandatory? It should be noted in the docs ...
> > 
> > > +          <optional>
> > > +            <attribute name="auto">
> > > +              <ref name="virYesNo"/>
> > > +            </attribute>
> > > +          </optional>
> > > +          <optional>
> > > +            <attribute name="cid">
> > > +              <ref name="unsignedInt"/>
> > > +            </attribute>
> > > +          </optional>
> > 
> > Also both attributes are optional which is kind of weird if <source> is
> > mandatory.
> > 
> > > +        </element>
> > > +        <optional>
> > > +          <ref name="address"/>
> > > +        </optional>
> > > +      </interleave>
> > > +    </element>
> > > +  </define>
> > 
> > [...]
> > 
> 
> [...]
> 
> > > diff --git a/tests/qemuxml2argvdata/vhost-vsock-auto.xml b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
> > > new file mode 100644
> > > index 0000000000..729d58bea4
> > > --- /dev/null
> > > +++ b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
> > > @@ -0,0 +1,35 @@
> > > +<domain type='qemu'>
> > > +  <name>test</name>
> > > +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
> > > +  <memory unit='KiB'>1048576</memory>
> > > +  <currentMemory unit='KiB'>1048576</currentMemory>
> > > +  <vcpu placement='static'>1</vcpu>
> > > +  <os>
> > > +    <type arch='x86_64' machine='pc-0.13'>hvm</type>
> > 
> > Our XMLs are living in the past.
> > 
> > > +    <boot dev='hd'/>
> > > +    <bootmenu enable='yes'/>
> > 
> > For an ACK please state how you plan to deal with the schema issues I've
> > pointed out. All other necessary changes were pointed out.
> 
> The idea is to transform <vsock> -> <vsock model='virtio'/> in the QEMU
> driver.
> 
> <source> and both of its attributes will be optional in the schema
> and in case 'auto' is missing, we'll tune it up in PostParse:
> 
>    if (vsock->auto_cid == VIR_TRISTATE_BOOL_ABSENT) {
>        if (vsock->guest_cid != 0)
>            vsock->auto_cid = VIR_TRISTATE_BOOL_NO;
>        else
>            vsock->auto_cid = VIR_TRISTATE_BOOL_YES;
>    }

Okay. ACK if you make <source> optional and add this code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180529/a3a7730c/attachment-0001.sig>