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

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(a)redhat.com&gt; >>> --- >>> 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.