1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/5] chardev source reconnect cleanup and fixes
  4. View patch

[libvirt] [PATCH 3/5] tests: don't use unix socket path that matches auto-generated path

Pavel Hrdina posted 5 patches 7 years, 8 months ago
[libvirt] [PATCH 3/5] tests: don't use unix socket path that matches auto-generated path
Posted by Pavel Hrdina 7 years, 8 months ago 
The test was introduced by 60135b22db6d.

The auto-generated path is removed by post-parse callback which
also changes the mode from "connect" to "bind" since the auto-generated
path makes sense only for "bind" mode.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args | 6 ++----
 tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml  | 4 ++--
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args
index 133a2c6039..8c6586e483 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args
@@ -23,14 +23,12 @@ server,nowait \
 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 \
 -device usb-ccid,id=ccid0,bus=usb.0,port=1 \
 -usb \
--chardev socket,id=charsmartcard0,path=/tmp/channel/domain-oldname/asdf,\
-reconnect=20 \
+-chardev socket,id=charsmartcard0,path=/tmp/channel/asdf,reconnect=20 \
 -device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \
 -chardev socket,id=charchannel0,host=localhost,port=1234,reconnect=10 \
 -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\
 id=channel0,name=asdf \
--chardev socket,id=charchannel1,path=/tmp/channel/domain--1-QEMUGuest1/fdsa,\
-server,nowait,reconnect=0 \
+-chardev socket,id=charchannel1,path=/tmp/channel/fdsa,reconnect=0 \
 -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\
 id=channel1,name=fdsa \
 -chardev socket,id=charredir0,host=localhost,port=3456,reconnect=15 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
index e0664b2a95..41ee248db3 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
@@ -18,7 +18,7 @@
       </source>
     </redirdev>
     <smartcard mode='passthrough' type='unix'>
-      <source mode='connect' path='/tmp/channel/domain-oldname/asdf'>
+      <source mode='connect' path='/tmp/channel/asdf'>
         <reconnect enabled='yes' timeout='20'/>
       </source>
     </smartcard>
@@ -29,7 +29,7 @@
       <target type='virtio' name='asdf'/>
     </channel>
     <channel type='unix'>
-      <source mode='connect' path='/tmp/channel/domain-oldname/fdsa'>
+      <source mode='connect' path='/tmp/channel/fdsa'>
         <reconnect enabled='no'/>
       </source>
       <target type='virtio' name='fdsa'/>
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] tests: don't use unix socket path that matches auto-generated path
Posted by Michal Privoznik 7 years, 8 months ago 
On 08/30/2017 01:40 PM, Pavel Hrdina wrote:
> The test was introduced by 60135b22db6d.
> 
> The auto-generated path is removed by post-parse callback which
> also changes the mode from "connect" to "bind" since the auto-generated
> path makes sense only for "bind" mode.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args | 6 ++----
>  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml  | 4 ++--
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 

> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> index e0664b2a95..41ee248db3 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> @@ -18,7 +18,7 @@
>        </source>
>      </redirdev>
>      <smartcard mode='passthrough' type='unix'>
> -      <source mode='connect' path='/tmp/channel/domain-oldname/asdf'>
> +      <source mode='connect' path='/tmp/channel/asdf'>
>          <reconnect enabled='yes' timeout='20'/>
>        </source>
>      </smartcard>
> @@ -29,7 +29,7 @@
>        <target type='virtio' name='asdf'/>
>      </channel>
>      <channel type='unix'>
> -      <source mode='connect' path='/tmp/channel/domain-oldname/fdsa'>
> +      <source mode='connect' path='/tmp/channel/fdsa'>
>          <reconnect enabled='no'/>
>        </source>
>        <target type='virtio' name='fdsa'/>
> 

This looks like it's fixing just a symptom not the cause. What if I have
a domain that has autogenerated path and I also set reconnect?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] tests: don't use unix socket path that matches auto-generated path
Posted by Pavel Hrdina 7 years, 8 months ago 
On Wed, Aug 30, 2017 at 02:33:00PM +0200, Michal Privoznik wrote:
> On 08/30/2017 01:40 PM, Pavel Hrdina wrote:
> > The test was introduced by 60135b22db6d.
> > 
> > The auto-generated path is removed by post-parse callback which
> > also changes the mode from "connect" to "bind" since the auto-generated
> > path makes sense only for "bind" mode.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args | 6 ++----
> >  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml  | 4 ++--
> >  2 files changed, 4 insertions(+), 6 deletions(-)
> > 
> 
> > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> > index e0664b2a95..41ee248db3 100644
> > --- a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> > +++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> > @@ -18,7 +18,7 @@
> >        </source>
> >      </redirdev>
> >      <smartcard mode='passthrough' type='unix'>
> > -      <source mode='connect' path='/tmp/channel/domain-oldname/asdf'>
> > +      <source mode='connect' path='/tmp/channel/asdf'>
> >          <reconnect enabled='yes' timeout='20'/>
> >        </source>
> >      </smartcard>
> > @@ -29,7 +29,7 @@
> >        <target type='virtio' name='asdf'/>
> >      </channel>
> >      <channel type='unix'>
> > -      <source mode='connect' path='/tmp/channel/domain-oldname/fdsa'>
> > +      <source mode='connect' path='/tmp/channel/fdsa'>
> >          <reconnect enabled='no'/>
> >        </source>
> >        <target type='virtio' name='fdsa'/>
> > 
> 
> This looks like it's fixing just a symptom not the cause. What if I have
> a domain that has autogenerated path and I also set reconnect?

If you try to define a domain with this configuration the post-parse
callback removes the path, see qemuDomainChrDefDropDefaultPath().  When
the guest is started there is another function,
see qemuDomainPrepareChannel(), where we generate a path in one is
missing and also changes the mode to "bind".

So yes, this fixes the symptom but it's still a valid fix because the
test XML is invalid, you should not use this generated path.

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] tests: don't use unix socket path that matches auto-generated path
Posted by Michal Privoznik 7 years, 8 months ago 
On 08/30/2017 02:45 PM, Pavel Hrdina wrote:
> On Wed, Aug 30, 2017 at 02:33:00PM +0200, Michal Privoznik wrote:
>> On 08/30/2017 01:40 PM, Pavel Hrdina wrote:
>>> The test was introduced by 60135b22db6d.
>>>
>>> The auto-generated path is removed by post-parse callback which
>>> also changes the mode from "connect" to "bind" since the auto-generated
>>> path makes sense only for "bind" mode.
>>>
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> ---
>>>  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args | 6 ++----
>>>  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml  | 4 ++--
>>>  2 files changed, 4 insertions(+), 6 deletions(-)
>>>
>>
>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
>>> index e0664b2a95..41ee248db3 100644
>>> --- a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
>>> @@ -18,7 +18,7 @@
>>>        </source>
>>>      </redirdev>
>>>      <smartcard mode='passthrough' type='unix'>
>>> -      <source mode='connect' path='/tmp/channel/domain-oldname/asdf'>
>>> +      <source mode='connect' path='/tmp/channel/asdf'>
>>>          <reconnect enabled='yes' timeout='20'/>
>>>        </source>
>>>      </smartcard>
>>> @@ -29,7 +29,7 @@
>>>        <target type='virtio' name='asdf'/>
>>>      </channel>
>>>      <channel type='unix'>
>>> -      <source mode='connect' path='/tmp/channel/domain-oldname/fdsa'>
>>> +      <source mode='connect' path='/tmp/channel/fdsa'>
>>>          <reconnect enabled='no'/>
>>>        </source>
>>>        <target type='virtio' name='fdsa'/>
>>>
>>
>> This looks like it's fixing just a symptom not the cause. What if I have
>> a domain that has autogenerated path and I also set reconnect?
> 
> If you try to define a domain with this configuration the post-parse
> callback removes the path, see qemuDomainChrDefDropDefaultPath().  When
> the guest is started there is another function,
> see qemuDomainPrepareChannel(), where we generate a path in one is
> missing and also changes the mode to "bind".
> 
> So yes, this fixes the symptom but it's still a valid fix because the
> test XML is invalid, you should not use this generated path.

Ah, right. You really want to fix just the test here.

ACK then and safe for the freeze.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] tests: don't use unix socket path that matches auto-generated path
Posted by Pavel Hrdina 7 years, 8 months ago 
On Wed, Aug 30, 2017 at 03:38:12PM +0200, Michal Privoznik wrote:
> On 08/30/2017 02:45 PM, Pavel Hrdina wrote:
> > On Wed, Aug 30, 2017 at 02:33:00PM +0200, Michal Privoznik wrote:
> >> On 08/30/2017 01:40 PM, Pavel Hrdina wrote:
> >>> The test was introduced by 60135b22db6d.
> >>>
> >>> The auto-generated path is removed by post-parse callback which
> >>> also changes the mode from "connect" to "bind" since the auto-generated
> >>> path makes sense only for "bind" mode.
> >>>
> >>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> >>> ---
> >>>  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.args | 6 ++----
> >>>  tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml  | 4 ++--
> >>>  2 files changed, 4 insertions(+), 6 deletions(-)
> >>>
> >>
> >>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> >>> index e0664b2a95..41ee248db3 100644
> >>> --- a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> >>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect.xml
> >>> @@ -18,7 +18,7 @@
> >>>        </source>
> >>>      </redirdev>
> >>>      <smartcard mode='passthrough' type='unix'>
> >>> -      <source mode='connect' path='/tmp/channel/domain-oldname/asdf'>
> >>> +      <source mode='connect' path='/tmp/channel/asdf'>
> >>>          <reconnect enabled='yes' timeout='20'/>
> >>>        </source>
> >>>      </smartcard>
> >>> @@ -29,7 +29,7 @@
> >>>        <target type='virtio' name='asdf'/>
> >>>      </channel>
> >>>      <channel type='unix'>
> >>> -      <source mode='connect' path='/tmp/channel/domain-oldname/fdsa'>
> >>> +      <source mode='connect' path='/tmp/channel/fdsa'>
> >>>          <reconnect enabled='no'/>
> >>>        </source>
> >>>        <target type='virtio' name='fdsa'/>
> >>>
> >>
> >> This looks like it's fixing just a symptom not the cause. What if I have
> >> a domain that has autogenerated path and I also set reconnect?
> > 
> > If you try to define a domain with this configuration the post-parse
> > callback removes the path, see qemuDomainChrDefDropDefaultPath().  When
> > the guest is started there is another function,
> > see qemuDomainPrepareChannel(), where we generate a path in one is
> > missing and also changes the mode to "bind".
> > 
> > So yes, this fixes the symptom but it's still a valid fix because the
> > test XML is invalid, you should not use this generated path.
> 
> Ah, right. You really want to fix just the test here.
> 
> ACK then and safe for the freeze.

Thanks, I've pushed the first two patches, I'll push this one as well.
For the last two patches I'll try to send a different approach that
would be safe for freeze.

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.