Bug 2103524

Summary:	Run virsh dumpxml cmd with extra options should return error
Product:	Red Hat Enterprise Linux 9	Reporter:	yalzhang <yalzhang>
Component:	libvirt	Assignee:	Michal Privoznik <mprivozn>
libvirt sub component:	General	QA Contact:	yalzhang <yalzhang>
Status:	CLOSED ERRATA	Docs Contact:
Severity:	unspecified
Priority:	unspecified	CC:	dzheng, jdenemar, lmen, mprivozn, pkrempa, virt-maint
Version:	9.1	Keywords:	Automation, Regression, Triaged, Upstream
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	libvirt-8.5.0-3.el9	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2022-11-15 10:04:39 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description yalzhang@redhat.com 2022-07-04 00:55:21 UTC

Description of problem:
Run virsh dumpxml cmd with extra options should return error

Version-Release number of selected component (if applicable):
libvirt-8.5.0-1.el9.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Run dumpxml with extra wrong argument, there is no outputs nor errors, and it returns '0':
# /usr/bin/virsh net-dumpxml default xyz

# echo $?
0

2. Try virsh dumpxml:
# virsh list
 Id   Name             State
--------------------------------
 1    avocado-vt-vm1   running

# virsh dumpxml 1 xxx

# echo $?
0

Actual results:
Step 1 & 2, run net-dumpxml and dumpxml with extra argument, the virsh command returns 0

Expected results:
It should return error

Additional info:
This is a regression, on libvirt-8.4.0-3.el9.x86_64:
1). virsh net-dumpxml 
#  /usr/bin/virsh net-dumpxml default  xyz
error: unexpected data 'xyz'
# echo $?
1
2). virsh dumpxml 
# virsh list
 Id   Name   State
----------------------
 11   rhel   running
# virsh dumpxml rhel xxx
error: unexpected data 'xxx'
# echo $?
1

Comment 2 Michal Privoznik 2022-07-04 13:11:19 UTC

I don't think this is a regression, it's a feature. With 8.5.0 virsh gained new feature: dumpxml [--domain] $dom [--xpath] XPATH

See https://gitlab.com/libvirt/libvirt/-/commit/8603b3d76ce54283812a9553da1b6f0e553a71f3

It's a convenient way to get only a subset of nodes instead of full domain XML.

Comment 3 Peter Krempa 2022-07-07 08:03:54 UTC

IMO the '--xpath' argument should have been declared with 'VSH_OFLAG_REQ_OPT' flag set making the '--xpath' prefix mandatory for the option which would also prevent eating any followign argument as the XPATH query.

While we've already released a libvirt version with the above code included I think we don't have enough users of it so we could change it to make the option name mandatory.

Comment 4 Michal Privoznik 2022-07-08 10:51:16 UTC

Fair enough, I've posted the fix here:

https://listman.redhat.com/archives/libvir-list/2022-July/232733.html

Comment 5 Michal Privoznik 2022-07-08 13:02:09 UTC

v2:

https://listman.redhat.com/archives/libvir-list/2022-July/232738.html

Comment 6 Michal Privoznik 2022-07-25 08:06:04 UTC

Merged upstream as:

commit e90d48ae6e22eaf1650f920abc0a6b87d2daa82b
Author:     Michal Prívozník <mprivozn>
AuthorDate: Fri Jul 8 12:45:42 2022 +0200
Commit:     Michal Prívozník <mprivozn>
CommitDate: Mon Jul 25 09:50:21 2022 +0200

    virsh: Require --xpath for *dumpxml
    
    Historically, the dumpxml command reject any unknown arguments,
    for instance:
    
        virsh dumpxml fedora xxx
    
    However, after v8.5.0-rc1~31 the second argument ('xxx') is
    treated as an XPath, but it's not that clearly visible.
    Therefore, require the --xpath switch, like this:
    
        virsh dumpxml fedora --xpath xxx
    
    Yes, this breaks already released virsh, but I think we can argue
    that the pool of users of this particular function is very small.
    We also document the argument being mandatory:
    
       dumpxml [--inactive] [--security-info] [--update-cpu] [--migratable]
               [--xpath EXPRESSION] [--wrap] domain
    
    The sooner we do this change, the better.
    
    The same applies for other *dumpxml functions (net-dumpxml,
    pool-dumpxml, vol-dumpxl to name a few).
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2103524
    Signed-off-by: Michal Privoznik <mprivozn>
    Reviewed-by: Peter Krempa <pkrempa>

v8.5.0-175-ge90d48ae6e

Comment 7 Michal Privoznik 2022-07-25 08:27:05 UTC

To POST:

https://gitlab.com/redhat/rhel/src/libvirt/-/merge_requests/38

Comment 9 yalzhang@redhat.com 2022-07-27 12:24:22 UTC

Test on libvirt-8.5.0-3.el9.x86_64, the result is as expected
# virsh net-dumpxml default xyz
error: unexpected data 'xyz'

# virsh net-dumpxml default --xpath xyz
# 
# virsh net-dumpxml default --xpath //ip
<ip address="192.168.122.1" netmask="255.255.255.0">
  <dhcp>
    <range start="192.168.122.2" end="192.168.122.254"/>
  </dhcp>
</ip>

Comment 12 yalzhang@redhat.com 2022-08-04 03:24:49 UTC

Test on libvirt-8.5.0-4.el9.x86_64, the result is as expected:
# virsh dumpxml rhel --xpath sdf

# virsh dumpxml rhel --xpath //os
<os firmware="efi">
  <type arch="x86_64" machine="pc-q35-rhel9.0.0">hvm</type>
  <boot dev="hd"/>
</os>

# virsh dumpxml rhel xyz
error: unexpected data 'xyz'

Comment 14 errata-xmlrpc 2022-11-15 10:04:39 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Low: libvirt security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2022:8003