tests/qemuxml2argvdata/aarch64-virt-graphics.xml | 1 - tests/qemuxml2argvdata/ppc64-pseries-graphics.xml | 1 - tests/qemuxml2argvdata/x86_64-pc-graphics.xml | 1 - tests/qemuxml2argvdata/x86_64-q35-graphics.xml | 1 - 4 files changed, 4 deletions(-)
The <source mode='bind'/> elements were added in commit 4d7ea75e and
apparently generated by virt-install, but since the @path attribute
is missing, the element will never be formatted back to the XML which the
libvirt-go-xml-check expects, so it fails.
The lack of the @path attribute is not a problem in general, because we
can autogenerate the path in certain cases, so qemuxml2argvtest will
always be happy, but XML formatting is a different thing.
Therefore, drop these generated elements, as they're unnecessary for the
purposes of qemuxml2argvtest and won't mess with the XML formatting
test in Go.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
Additionally, I could introduce a xml2xml test here, but I already spent enough
time hunting the CI failure down between libvirt and libvirt-go so this was the
simplest solution which doesn't require any extra work.
tests/qemuxml2argvdata/aarch64-virt-graphics.xml | 1 -
tests/qemuxml2argvdata/ppc64-pseries-graphics.xml | 1 -
tests/qemuxml2argvdata/x86_64-pc-graphics.xml | 1 -
tests/qemuxml2argvdata/x86_64-q35-graphics.xml | 1 -
4 files changed, 4 deletions(-)
diff --git a/tests/qemuxml2argvdata/aarch64-virt-graphics.xml b/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
index 95aef91beb..2559cd7950 100644
--- a/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
+++ b/tests/qemuxml2argvdata/aarch64-virt-graphics.xml
@@ -34,7 +34,6 @@
</interface>
<console type="pty"/>
<channel type="unix">
- <source mode="bind"/>
<target type="virtio" name="org.qemu.guest_agent.0"/>
</channel>
<input type="tablet" bus="usb"/>
diff --git a/tests/qemuxml2argvdata/ppc64-pseries-graphics.xml b/tests/qemuxml2argvdata/ppc64-pseries-graphics.xml
index 3d54a4f171..bab115f666 100644
--- a/tests/qemuxml2argvdata/ppc64-pseries-graphics.xml
+++ b/tests/qemuxml2argvdata/ppc64-pseries-graphics.xml
@@ -28,7 +28,6 @@
</interface>
<console type="pty"/>
<channel type="unix">
- <source mode="bind"/>
<target type="virtio" name="org.qemu.guest_agent.0"/>
</channel>
<input type="tablet" bus="usb"/>
diff --git a/tests/qemuxml2argvdata/x86_64-pc-graphics.xml b/tests/qemuxml2argvdata/x86_64-pc-graphics.xml
index 03745eabf4..8d8fc14f9f 100644
--- a/tests/qemuxml2argvdata/x86_64-pc-graphics.xml
+++ b/tests/qemuxml2argvdata/x86_64-pc-graphics.xml
@@ -40,7 +40,6 @@
</interface>
<console type="pty"/>
<channel type="unix">
- <source mode="bind"/>
<target type="virtio" name="org.qemu.guest_agent.0"/>
</channel>
<input type="tablet" bus="usb"/>
diff --git a/tests/qemuxml2argvdata/x86_64-q35-graphics.xml b/tests/qemuxml2argvdata/x86_64-q35-graphics.xml
index 56db898e64..a93b49e480 100644
--- a/tests/qemuxml2argvdata/x86_64-q35-graphics.xml
+++ b/tests/qemuxml2argvdata/x86_64-q35-graphics.xml
@@ -40,7 +40,6 @@
</interface>
<console type="pty"/>
<channel type="unix">
- <source mode="bind"/>
<target type="virtio" name="org.qemu.guest_agent.0"/>
</channel>
<input type="tablet" bus="usb"/>
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2019-03-13 at 09:42 +0100, Erik Skultety wrote: > The <source mode='bind'/> elements were added in commit 4d7ea75e and > apparently generated by virt-install, but since the @path attribute > is missing, the element will never be formatted back to the XML which the > libvirt-go-xml-check expects, so it fails. > The lack of the @path attribute is not a problem in general, because we > can autogenerate the path in certain cases, so qemuxml2argvtest will > always be happy, but XML formatting is a different thing. > > Therefore, drop these generated elements, as they're unnecessary for the > purposes of qemuxml2argvtest and won't mess with the XML formatting > test in Go. > > Signed-off-by: Erik Skultety <eskultet@redhat.com> > --- > > Additionally, I could introduce a xml2xml test here, but I already spent enough > time hunting the CI failure down between libvirt and libvirt-go so this was the > simplest solution which doesn't require any extra work. I'm okay with the change, so Reviewed-by: Andrea Bolognani <abologna@redhat.com> but I wonder if this is the expected behavior for the Go XML parsing and formatting code? IIUC from your description the test is basically parsing the input file and formatting it back, and I would expect the process to be either * entirely dumb, so that each element and attribute that was present in the input file will show up in the output file as well; or * as smart as libvirt's qemuxml2xmltest, which means the input file will go through the whole postParse() rigmarole and come out on the other side significantly altered. It sounds like this test is mostly the former, but in this specific case there is a little bit of logic sprinkled in? And somehow it only affects the <source> element, because no other input file up to this point caused issues? CC'ing Dan who will certainly have some insights. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 13, 2019 at 10:04:27AM +0100, Andrea Bolognani wrote: > On Wed, 2019-03-13 at 09:42 +0100, Erik Skultety wrote: > > The <source mode='bind'/> elements were added in commit 4d7ea75e and > > apparently generated by virt-install, but since the @path attribute > > is missing, the element will never be formatted back to the XML which the > > libvirt-go-xml-check expects, so it fails. > > The lack of the @path attribute is not a problem in general, because we > > can autogenerate the path in certain cases, so qemuxml2argvtest will > > always be happy, but XML formatting is a different thing. > > > > Therefore, drop these generated elements, as they're unnecessary for the > > purposes of qemuxml2argvtest and won't mess with the XML formatting > > test in Go. > > > > Signed-off-by: Erik Skultety <eskultet@redhat.com> > > --- > > > > Additionally, I could introduce a xml2xml test here, but I already spent enough > > time hunting the CI failure down between libvirt and libvirt-go so this was the > > simplest solution which doesn't require any extra work. > > I'm okay with the change, so > > Reviewed-by: Andrea Bolognani <abologna@redhat.com> > > but I wonder if this is the expected behavior for the Go XML > parsing and formatting code? So, since Dan fixed it properly within libvirt-go-xml with commit c834a236, this is a sNACK then. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 13, 2019 at 02:29:08PM +0100, Erik Skultety wrote: > On Wed, Mar 13, 2019 at 10:04:27AM +0100, Andrea Bolognani wrote: > > On Wed, 2019-03-13 at 09:42 +0100, Erik Skultety wrote: > > > The <source mode='bind'/> elements were added in commit 4d7ea75e and > > > apparently generated by virt-install, but since the @path attribute > > > is missing, the element will never be formatted back to the XML which the > > > libvirt-go-xml-check expects, so it fails. > > > The lack of the @path attribute is not a problem in general, because we > > > can autogenerate the path in certain cases, so qemuxml2argvtest will > > > always be happy, but XML formatting is a different thing. > > > > > > Therefore, drop these generated elements, as they're unnecessary for the > > > purposes of qemuxml2argvtest and won't mess with the XML formatting > > > test in Go. > > > > > > Signed-off-by: Erik Skultety <eskultet@redhat.com> > > > --- > > > > > > Additionally, I could introduce a xml2xml test here, but I already spent enough > > > time hunting the CI failure down between libvirt and libvirt-go so this was the > > > simplest solution which doesn't require any extra work. > > > > I'm okay with the change, so > > > > Reviewed-by: Andrea Bolognani <abologna@redhat.com> > > > > but I wonder if this is the expected behavior for the Go XML > > parsing and formatting code? > > So, since Dan fixed it properly within libvirt-go-xml with commit c834a236, > this is a sNACK then. Yeah the go xml code was a screwup on my part as I never realized it was valid to have a empty path. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.