Fix potential deadlock across fork() in QEMU driver

The hook scripts used by virCommand must be careful wrt
accessing any mutexes that may have been held by other
threads in the parent process. With the recent refactoring
there are 2 potential flaws lurking, which will become real
deadlock bugs once the global QEMU driver lock is removed.

Remove use of the QEMU driver lock from the hook function
by passing in the 'virQEMUDriverConfigPtr' instance directly.

Add functions to the virSecurityManager to be invoked before
and after fork, to ensure the mutex is held by the current
thread. This allows it to be safely used in the hook script
in the child process.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

build: fix compilation of selinux on RHEL 5

On RHEL 5, I got:

security/security_selinux.c: In function 'getContext':
security/security_selinux.c:971: warning: unused parameter 'mgr' [-Wunused-parameter]

* src/security/security_selinux.c (getContext): Mark potentially
unused parameter.

S390: domain_conf support for CCW

Add necessary handling code for the new s390 CCW address type to
virDomainDeviceInfo. Further, introduce  memory management, XML
parsing, output formatting and range validation for the new
virDomainDeviceCCWAddress type.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

S390: Documentation for CCW address type

The native bus for s390 I/O is called CCW (channel command word).
As QEMU has added basic support for the CCW bus, i.e. the
ability to assign CCW devnos (bus addresses) to devices.
Domains with the new machine type s390-ccw-virtio can use the
CCW bus. Currently QEMU will only allow to define virtio
devices on the CCW bus.
Here we add the new machine type and the new device address to the
schema definition and add a new paragraph to the domain XML
documentation.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

build: fix VPATH testsuite

'make check' has been failing on VPATH builds since commit
907a39e7.  The tests already had magic for munging path names,
but were munging to the wrong location, thus working only on
an in-tree build.

* tests/securityselinuxlabeltest.c (testSELinuxMungePath): Munge
to correct path.

docs: fix 1.0.2 release date

* docs/news.html.in: Use correct release year.

build: fix make check of remote_protocol-structs

Broken by incorrect formatting / spelling of remote_nonnull in commit
39758e7567b766f1df3948ea671d6ccb93daf7a9

Check if classes are derived from object

This makes sure we don't regress to old style classes

hypervisor: Restore pm initialization

Adjustment for 'c059cdeaf' due to older compiler complaint about pm
not being initialized even though the j&7 == 0 does the trick.

virsh: Use virNodeDeviceLookupSCSIHostByWWN

Only nodedev-destroy and nodedev-dumpxml can benifit from the
new API, other commands like nodedev-detach only works for
PCI devices, WWN makes no sense for them.

nodedev: Implement virNodeDeviceLookupSCSIHostByWWN

This just simply changes nodeDeviceLookupByWWN to be not static,
and its name into nodeDeviceLookupSCSIHostByWWN. And use that for
udev and HAL backends.

remote: Wire up the remote protocol

Like virNodeDeviceCreateXML, virNodeDeviceLookupSCSIHostByWWN
has to be treated specially when generating the RPC codes. Also
new rules are added in fixup_name to keep the name SCSIHostByWWN.

Introduce API virNodeDeviceLookupSCSIHostByWWN

Since the name (like scsi_host10) is not stable for vHBA, (it can
be changed either after recreating or system rebooting), current
API virNodeDeviceLookupByName is not nice to use for management app
in this case. (E.g. one wants to destroy the vHBA whose name has
been changed after system rebooting, he has to find out current
name first).

Later patches will support the persistent vHBA via storage pool,
with which one can identify the vHBA stably by the wwnn && wwpn
pair.

So this new API comes.

Remove re-entrant API call in SELinux/AppArmor security managers

The security manager drivers are not allowed to call back
out to top level security manager APIs, since that results
in recursive mutex acquisition and thus deadlock. Remove
calls to virSecurityManagerGetModel from SELinux / AppArmor
drivers

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

hypervisor: Revisit Coverity issues regarding cpumap

Turns out the issue regarding ptr_arith and sign_exension weren't false
positives. When shifting an 'unsigned char' as a target, it gets promoted
to an 'int'; however, that 'int' cannot be shifted 32 bits which was how
the algorithm was written. For the ptr_arith rather than index into the
cpumap, change the to address as necessary and assign directly.

hypervisor: Remove redundant validity checks, clean up function headers

Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.

xend: Fix a memory leak found by Coverity

Commit id '87b4c10c' moved the VIR_ALLOC_N, but didn't check if 'cpuset'
had been allocated on failure.

xend: Remove redundant validity checks, clean up function headers

Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.

xm: Remove redundant validity checks, clean up function headers

Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent. Don't type caste the
privateData.

inotify: Clean up some function headers

xs:  Remove redundant validity checks, clean up function headers

Arguments for driver entry points are checked in libvirt.c, so no need to
check again. Make function entry points consistent.

virsh-snapshot: Reject --no-metadata together with --print-xml

Manual for "virsh snapshot-create-as" states that --no-metadata and
--print-xml are incompatible. Honor this detail in the code.

virsh-snapshot: Refactor some details in virsh snapshot-create-as

This patch simplifies the creation of XML, some error paths and adds
correct approach to check for virBuffer errors.

Pass virQEMUDriverPtr into APIs managed shared disk list

Currently the APIs for managing the shared disk list take
a virHashTablePtr as the primary argument. This is bad
because it requires the caller to deal with locking of
the QEMU driver. Switch the APIs to take the full
virQEMUDriverPtr instance

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Serialize execution of security manager APIs

Add locking to virSecurityManagerXXX APIs, so that use of the
security drivers is internally serialized. This avoids the need
to rely on the global driver locks to achieve serialization

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Turn virSecurityManager into a virObjectLockable

To enable locking to be introduced to the security manager
objects later, turn virSecurityManager into a virObjectLockable
class

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

sheepdog: skip refresh on creation failure

Don't try to refresh Sheepdog volume if creating volume fails.

Signed-off-by: Harry Wei <harryxiyou@gmail.com>

util: refactor iptables command construction into multiple steps

Instead of creating an iptables command in one shot, do it in steps
so we can add conditional options like physdev and protocol.

This removes code duplication while keeping existing behaviour.

Signed-off-by: Natanael Copa <ncopa@alpinelinux.org>
Signed-off-by: Eric Blake <eblake@redhat.com>

qemu: support vhost-net for generic ethernet devices

From qemu's point of view these are still just tap devices, so there's
no reason they shouldn't work with vhost-net; as a matter of fact,
Raja Sivaramakrishnan <srajag00@yahoo.com> verified on libvir-list
that at least the qemu_command.c part of this patch works:

  https://www.redhat.com/archives/libvir-list/2012-December/msg01314.html

(the hotplug case is extrapolation on my part).

network_conf.c: Free xmlDoc after use

The virNetworkObjUpdateParseFile() function was not freeing the xml
variable, leaving us with a memory leak.

virsh-snapshot: Fix XPath query to determine snapshot state

The query didn't match the external state correctly for offline internal
snapshots.

Stop accessing driver->caps directly in QEMU driver

The 'driver->caps' pointer can be changed on the fly. Accessing
it currently requires the global driver lock. Isolate this
access in a single helper, so a future patch can relax the
locking constraints.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Rename 'qemuCapsXXX' to 'virQEMUCapsXXX'

To avoid confusion between 'virCapsPtr' and 'qemuCapsPtr'
do some renaming of various fucntions/variables. All
instances of 'qemuCapsPtr' are renamed to 'qemuCaps'. To
avoid that clashing with the 'qemuCaps' typedef though,
rename the latter to virQEMUCaps.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Turn virCapabilities into a virObject

To enable virCapabilities instances to be reference counted,
turn it into a virObject. All cases of virCapabilitiesFree
turn into virObjectUnref

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Fix comment about virCgroupPtr locking rules in QEMU driver

The virCgroupPtr instance APIs are safe to use without locking
in the QEMU driver, since all internal state they rely on is
immutable. Update the comment to reflect this.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Fix PKI directory used for QEMU test suite

The data files for testing QEMU command line generation are
hardcoded to use /etc/pki, so we should explicitly set that
in the test case, avoiding the dynamic SYSCONFDIR value.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

virCondDestroy: Lose attribute RETURN_CHECK

We are wrapping it in ignore_value() anyway.

Drop useless virFileWrapperFdCatchError

We are requesting for stderr catching for all cases in
virFileWrapperFdNew(). There is no need to have a separate
function just to report an error, esp. when we can do it in
virFileWrapperFdClose().

HACKING: Sync with docs/hacking.html.in

hacking: Add some details to handle Valgrind output

hacking: Add some text around the running of Valgrind along with example
output for "real" vs. "false positives".

cfg.mk: Add hacking.in.html to sc_prohibit_raw_allocation

valgrind: Adjust the suppression file

virnetttlcontext: Resolve issues found by Valgrind

Need to initialize 'usage' and 'critical' since the VIR_DEBUG will
attempt to use them.

qemumonitortestutils: Resolve resource leaks found by Valgrind

When Valgrind runs the 'qemumonitorjsontest' it would claim that the
thread created is leaked. That's because the virThreadJoin won't get
called due to the 'running' flag being cleared.  In order to avoid that,
call virThreadJoin unconditionally at cleanup time.  Also noted that the
qemuMonitorTestWorker() didn't get the test mutex lock on the failure path.

The incoming and outgoing buffers allocated by qemuMonitorTestIO() and
qemuMonitorTestAddReponse() were never VIR_FREE()'d in qemuMonitorTestFree().

qemumonitorjsontest: Resolve resource leaks found by Valgrind

The 'package' string returned by qemuMonitorGetVersion() needs to
be VIR_FREE()'d.

testQemuMonitorJSONGetMachines(), testQemuMonitorJSONGetCPUDefinitions(),
and testQemuMonitorJSONGetCommands() did not VIR_FREE() the array and
array elements allocated by their respective qemuMonitorGet* routines.

qemu_command: Resolve resource leaks found by Valgrind

The qemuParseGlusterString() replaced dst->src without a VIR_FREE() of
what was in there before.

The qemuBuildCommandLine() did not properly free the boot_buf depending
on various usages.

The qemuParseCommandLineDisk() had numerous paths that didn't clean up
the virDomainDiskDefPtr def properly. Adjust the logic to go through an
error: label before cleanup in order to free the resource.

domain_conf: Resolve resource leaks found by Valgrind

Fix various resource leaks discovered while parsing through Valgrind output

vport_profile_conf: Resolve memory leak found by Valgrind

The 'virtPortInterfaceID' was not VIR_FREE()'d

netdev_vlan_conf: Resolve memory leak found by Valgrind.

The 'trunk' is filled in with virXPathString() value, but was
never VIR_FREE()'d.

qemuxml2argvtest: Resolve resource leaks found by Valgrind

Valgrind deterimined that fakeSecretGetValue() was using the secret
value without checking validity.  Returning NULL causes the caller
to emit a message and results in failure.

Additionally commit 'b090aa7d' changes leaked vncSASLdir and vncTLSx509certdir

virnettlscontexttest: Resolve memory leak found by Valgrind

testTLSDerEncode() will allocate memory for der.data, it wasn't VIR_FREE()'d.
also don't initialized der to use static buffer.

Add syntax-check to make sure Python files don't contain trailing semicolon

We allow for a trailing semicolon in full line comments since
docs/index.py has some SQL statements in it.

Remove more trailing semicolons in Python files

syntax-check: Don't check non-reentrant functions in docs

Otherwise constructions like "random (used by default)" end up breaking
syntax check by apparently using non-reentrant functions.

Cosmetics: Remove semicolons

It's Python, not C

build: fix build of HAL node backend

Commit 2025356 missed uses of PCI functions in the older HAL-related
code, probably because hal-devel is no longer available in latest Fedora.

* src/node_device/node_device_hal.c (gather_pci_cap): Reflect
function rename.

bitmap: add way to find next clear bit

We had an easy way to iterate set bits, but not for iterating
cleared bits.

* src/util/virbitmap.h (virBitmapNextClearBit): New prototype.
* src/util/virbitmap.c (virBitmapNextClearBit): Implement it.
* src/libvirt_private.syms (bitmap.h): Export it.
* tests/virbitmaptest.c (test4): Test it.

qemu_hotplug: Need to call virUSBDeviceFree()

rpc: Need to virCommandFree on error path

uml: Resolve leak if need to requery in umlIdentifyOneChrPTY()

Coverity noted that in the retry logic loop if res had been set, then
it could be leaked so add a VIR_FREE(res) prior to retry.

openvz: Need to error check openvzDomainSetVcpusFlagsInternal()

parallels: Need to free memory on error path

network: Remove conditional settings to resolve resource leak

The conditional setting of cmdout in networkBuildDhcpDaemonCommandLine()
caused Coverity to complain that 'cmd' could be leaked if !cmdout.  Since
the function is local and only called with cmdout being passed those checks
have been removed.

storage: Resolve resource leaks with cmd processing

keepalive: Resolve Coverity complaint

The Coverity analysis emitted a BAD_SIZEOF error when doing the math
within the TRACE macro. Doing the math outside the macro keeps Coverity quiet.

tlscontext: Make sure to get proper pointer to 'dname'

Keep Coverity happy by passing a pointer to 'dname' rather than the
array itself.  The PROBE expansion would cause a BAD_SIZEOF.

sanitytest.py: Do not rely on system libvirt

When running sanitytest.py we should not rely on libvirt library
installed on the system. And since we generate a nice wrapper called
"run" that sets both PYTHON_PATH and LD_LIBRARY_PATH, we should just use
it rather than trying to duplicate it in the Makefile.

Initialize qemuImageBinary path at startup

Introduce annotations for virQEMUDriverPtr fields

Annotate the fields in virQEMUDriverPtr to indicate the locking
rules for their use

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Protect USB/PCI device list access in QEMU with dedicated locks

Currently the activePciHostdevs, inactivePciHostdevsd and
activeUsbHostdevs lists are all implicitly protected by the
QEMU driver lock. Now that the lists all inherit from the
virObjectLockable, we can make the locking explicit, removing
the dependency on the QEMU driver lock for correctness.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Convert virPCIDeviceList and virUSBDeviceList into virObjectLockable

To allow modifications to the lists to be synchronized, convert
virPCIDeviceList and virUSBDeviceList into virObjectLockable
classes. The locking, however, will not be self-contained. The
users of these classes will have to call virObjectLock/Unlock
in the critical regions.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Rename all USB device functions to have a standard name prefix

Rename all the usbDeviceXXX and usbXXXDevice APIs to have a
fixed virUSBDevice name prefix

Fix leak of usbDevice struct when initializing cgroups

When iterating over USB host devices to setup cgroups, the
usbDevice object was leaked in both LXC and QEMU driers

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Rename all PCI device functions to have a standard name prefix

Rename all the pciDeviceXXX and pciXXXDevice APIs to have a
fixed virPCIDevice name prefix

Remove pointless 'qemuVersion' field from virQEMUDriverPtr

The QEMU driver struct has a 'qemuVersion' field that was previously
used to cache the version lookup from capabilities. With the recent
QEMU capabilities rewrite the caching happens at a lower level so
this field is pointless. Removing it avoids worries about locking
when updating it.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Use atomic ops to increment nextvmid

Use atomic ops to increment nextvmid and encapsulate it in a
method to prevent accidental non-atomic access

Make virDomainObjList self-locking via virObjectLockable

Switch virDomainObjList to inherit from virObjectLockable and
make all the APIs acquire/release the mutex when running. This
makes virDomainObjList completely self-locking and no longer
reliant on the hypervisor driver locks

Merge virDomainObjListIsDuplicate into virDomainObjListAdd

The duplicate VM checking should be done atomically with
virDomainObjListAdd, so shoud not be a separate function.
Instead just use flags to indicate what kind of checks are
required.

This pair, used in virDomainCreateXML:

   if (virDomainObjListIsDuplicate(privconn->domains, def, 1) < 0)
     goto cleanup;
   if (!(dom = virDomainObjListAdd(privconn->domains,
                                   privconn->caps,
                                   def, false)))
     goto cleanup;

Changes to

   if (!(dom = virDomainObjListAdd(privconn->domains,
                                   privconn->caps,
                                   def,
                                   VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                   NULL)))
     goto cleanup;

This pair, used in virDomainRestoreFlags:

   if (virDomainObjListIsDuplicate(privconn->domains, def, 1) < 0)
     goto cleanup;
   if (!(dom = virDomainObjListAdd(privconn->domains,
                                   privconn->caps,
                                   def, true)))
     goto cleanup;

Changes to

   if (!(dom = virDomainObjListAdd(privconn->domains,
                                   privconn->caps,
                                   def,
                                   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
                                   VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                   NULL)))
     goto cleanup;

This pair, used in virDomainDefineXML:

   if (virDomainObjListIsDuplicate(privconn->domains, def, 0) < 0)
     goto cleanup;
   if (!(dom = virDomainObjListAdd(privconn->domains,
                                   privconn->caps,
                                   def, false)))
     goto cleanup;

Changes to

   if (!(dom = virDomainObjListAdd(privconn->domains,
                                   privconn->caps,
                                   def,
                                   0, NULL)))
     goto cleanup;

Fix typo in configure.ac causing $LIBS to gain a copy of $CFLAGS

The virt-dbus.m4 check for DBus was preserving $LIBS before
modifying it. Except it wasn't. It was preserving another
copy of $CFLAGS. The result was that after the check completed,
$LIBS got polluted with $CFLAGS

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

tests: reserve more fds for commandtest

Commit 39c77fe triggered random failures, depending on the platform
and what other fds leak into the testsuite (for me, it passed on
RHEL 6 but failed on Fedora 18).  The reason was that we were
expecting an fd that fell outside of our reserved range.  By reserving
a larger range, the test once again passes on all platforms.

* tests/commandtest.c (mymain): Reserve enough fds.

qemu: don't log failure during QMP add-fd probe

Otherwise, we get a lot of scary (but harmless) noise in the logs:

2013-02-05 15:35:48.555+0000: 8637: error : qemuMonitorJSONCheckError:353 : internal error unable to execute QEMU command 'add-fd': Parameter 'fdset-id' expects an existing fdset-id

one for every qemu 1.2 binary that we probe.

* src/qemu/qemu_monitor_json.c (qemuMonitorJSONAddFd): During
probe, avoid logging failures.

Add John Ferlan to the committers list

Turn virDomainObjList into an opaque virObject

As a step towards making virDomainObjList thread-safe turn it
into an opaque virObject, preventing any direct access to its
internals.

As part of this a new method virDomainObjListForEach is
introduced to replace all existing usage of virHashForEach

Rename all domain list APIs to have virDomainObjList prefix

The APIs names for accessing the domain list object are
very inconsistent. Rename them all to have a standard
virDomainObjList prefix.

Introduce a virQEMUDriverConfigPtr object

Currently the virQEMUDriverPtr struct contains an wide variety
of data with varying access needs. Move all the static config
data into a dedicated virQEMUDriverConfigPtr object. The only
locking requirement is to hold the driver lock, while obtaining
an instance of virQEMUDriverConfigPtr. Once a reference is held
on the config object, it can be used completely lockless since
it is immutable.

NB, not all APIs correctly hold the driver lock while getting
a reference to the config object in this patch. This is safe
for now since the config is never updated on the fly. Later
patches will address this fully.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Fix missing error constants in libvirt python module

The previous change to the generator, changed too much - only
the functions are in 'virerror.c', the constants remained in
'virerror.h' which could not be renamed for API compat reasons.

Add a test case to sanity check the generated python bindings

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

qemu: Catch stderr of image compression binary

If a compression binary prints something to stderr, currently
it is discarded. However, it can contain useful data from
debugging POV, so we should catch it.

qemu: Catch stderr of image decompression binary

If a decompression binary prints something to stderr, currently
it is discarded. However, it can contain useful data from
debugging POV, so we should catch it.

virFileWrapperFd: Switch to new virCommandDoAsyncIO

Commit 34e8f63a32f83 introduced support for catching errors from
libvirt iohelper. However, at those times there wasn't such fancy
API as virCommandDoAsyncIO(), so everything has to be implemented
on our own. But since we do have the API now, we can use it and
drop our implementation then.

tests: Create test for virCommandDoAsyncIO

This is just a basic test, so we don't break virCommand in the
future. A "Hello world\n" string is written to commanhelper,
which copies input to stdout and stderr where we read it from.
Then the read strings are compared with expected values.

Introduce event loop to commandtest

This is just preparing environment for the next patch, which is
going to need an event loop.

virCommand: Introduce virCommandDoAsyncIO

Currently, if we want to feed stdin, or catch stdout or stderr of a
virCommand we have to use virCommandRun(). When using virCommandRunAsync()
we have to register FD handles by hand. This may lead to code duplication.
Hence, introduce an internal API, which does this automatically within
virCommandRunAsync(). The intended usage looks like this:

    virCommandPtr cmd = virCommandNew*(...);
    char *buf = NULL;

    ...

    virCommandSetOutputBuffer(cmd, &buf);
    virCommandDoAsyncIO(cmd);

    if (virCommandRunAsync(cmd, NULL) < 0)
        goto cleanup;

    ...

    if (virCommandWait(cmd, NULL) < 0)
        goto cleanup;

    /* @buf now contains @cmd's stdout */
    VIR_DEBUG("STDOUT: %s", NULLSTR(buf));

    ...

cleanup:
    VIR_FREE(buf);
    virCommandFree(cmd);

Note, that both stdout and stderr buffers may change until virCommandWait()
returns.

conf: Use correct type for device type enum in virDomainDeviceDefFree

With this change it's easy to spot a forgotten free if a new device
class is added.

build: Add libcurl dependency to libvirt_driver.la

libvirt.c calls curl_global_init() if WITH_CURL is defined and thus it
should be linked with libcurl. This fixes link failure in case neither
xenapi nor esx driver is enabled (they are the only users of libcurl).

spec: Fix minor changelog issues

When a changelog entry references an RPM macro, % needs to be escaped so
that it does not appear expanded in package changelog.

Fri Mar  4 2009 is incorrect since Mar 4 was Wednesday. Since
libvirt-0.6.1 was released on Mar 4 2009, we should change Fri to Wed.

Add basic support for VDI images

QEMU is fully capable of handling VDI images and we just refuse to
work with them.  As qemu-img knows and supports this, there should be
no problem with this addition.

This is of course, just basic functionality, without searching for any
backing files, etc.

Support shifted magic in storage files

Some files have the magic shifted to some offset other than 0, so we
have to support that.  I also cleaned up some lines to be more
readable and added missing magic for iso file format.

virsh-secret: Refactor error paths

This patch switches string option retrieval to vshCommandOptStringReq
and refactors some error paths to avoid an unlikely memory leak of a
secret object in cmdSecretSetValue.

virsh-nwfilter: Update nwfilter commands to use vshCommandOptStringReq

virsh-snapshot: Update snapshot commands to use vshCommandOptStringReq

also avoids potential NULL pointer dereference:

$ virsh snapshot-current asdf ""
error: invalid snapshotname argument '(null)'

by removing the error message in favor of vshCommandOptStringReq

virsh-snapshot: Refactor cmdSnapshotDumpXML

This patch simplifies error paths and switches to use
vshCommandOptStringReq for argument retrieval in cmdSnapshotDumpXML