With this patch, it is hopefully a bit more obvious that for
snapshot-create-as, a literal '--diskspec' is mandatory if name
or description was omitted, but optional if all earlier options
were provided.
These all denote two diskspecs and a description:
virsh snapshot-create-as dom name desc vda vdb
virsh snapshot-create-as dom name desc --diskspec vda --diskspec vdb
virsh snapshot-create-as dom name desc --diskspec vda vdb
virsh snapshot-create-as dom name desc vda --diskspec vdb
virsh snapshot-create-as dom --diskspec vda --diskspec vdb name desc
This gives two diskspecs but no description:
virsh snapshot-create-as dom name --diskspec vda --diskspec vdb
And this treats 'vda' as the description, with only one diskspec:
virsh snapshot-create-as dom name vda vdb
The help output now shows:
snapshot-create-as <domain> [<name>] [<description>] [--print-xml] [--no-metadata] [--halt] [--disk-only] [[--diskspec] <string>]...
I also checked the help output for echo and send-key, which are two
other variants of argv commands.
* tools/virsh.pod (snapshot-create-as): Document when a literal
--diskspec must preceed a diskspec argument.
* tools/virsh.c (vshCmddefHelp): Update help output for argv when
naming the option is useful.
(vshCmddefGetData): Fix logic on when argv was seen.
* tests/virsh-optparse: Add tests to avoid regressions.
Commit 498d783 cleans up some of virtual file names for parsing strings
in memory. This patch cleans up (hopefuly) the rest forgotten by the
first patch.
This patch also changes all of the previously modified "filenames" to
valid URI's replacing spaces for underscores.
Changes to v1:
- Replace all spaces for underscores, so that the strings form valid
URI's
- Replace spaces in places changed by commit 498d783
Commit ffe28ab74b introduced regression
while communicating with older libvirtd command 'domblkstat' used the new
API and did not check for VIR_ERR_RPC error code signalling the remote
server does not support this API and did not fall back to older API.
Thereafter 'domblkstat' ended with "error: unknown procedure: 243".
While parsing XML strings from memory, the previous convention in
libvirt was to set the virtual file name to "domain.xml" or something
similar. This could potentialy trick the user into looking for a file
named domain.xml on the disk in an attempt to fix the error.
This patch changes these filenames to something that can't be as easily
confused for a valid filename.
Examples of error messages:
---------------------------
Error while loading file from disk:
15:07:59.015: 527: error : catchXMLError:709 : /path/to/domain.xml:1: StartTag: invalid element name
<domain type='kvm'><
--------------------^
Error while parsing definition in memory:
15:08:43.581: 525: error : catchXMLError:709 : (domain definition):2: error parsing attribute name
<name>vm1</name>
--^
This has the added benefit of making 'snapshot-create dom --no-metadata'
now able to tell you the name of the just-generated snapshot.
* tools/virsh.c (vshSnapshotCreate, cmdSnapshotCurrent): Don't get
XML just for name.
Two new commands are added to virsh that wrap usage of
virDomainUpdateDeviceFlags for changing link state of domain's network
interfaces. These wrappers extract network devices's xml configuration
and modify the link state for easy manipulation from an user's perspective.
- domif-setlink - set link state of a domains virtual network interface
- domif-getlink - get link state
* tools/virsh.c - Add functionality to virsh
* tools/virsh.pod - Manpage documentation
The modified function fallbacks to use virDomainBlockStats if
virDomainBlockStatsFlags is not supported by the hypervisor driver.
If the new API is supported, it will be invoked instead of the
old API.
Expose the disk-only flag through virsh. Additionally, make
virsh snapshot-create-as take an arbitrary number of diskspecs,
which can be used to build up the xml for <domainsnapshot>.
* tools/virsh.c (cmdSnapshotCreate): Add --disk-only.
(cmdSnapshotCreateAs): Likewise, and add argv diskspec.
(vshParseSnapshotDiskspec): New helper function.
(vshCmddefGetOption): Allow naming of argv field.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
them.
* tests/virsh-optparse: Test snapshot-create-as parsing.
This adds a convenience function to virsh that parses out block
information from the domain xml, making it much easier to see
what strings can be used in all other contexts that demand a
specific block name, especially when given the previous patch
that allows using either target or unique source name.
As an example on a domain with one disk and an empty cdrom drive:
Target Source
-------------------------------------------
vda /var/lib/libvirt/images/fedora_12.img
hdc -
* tools/virsh.c (cmdDomblklist): New function.
* tools/virsh.pod (domblklist): Document it.
In order to distinguish disk snapshots from system checkpoints, a
new state value that is only valid for snapshots is helpful.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_LAST): New placeholder.
* src/conf/domain_conf.h (virDomainSnapshotState): New enum mapping.
(VIR_DOMAIN_DISK_SNAPSHOT): New internal enum value.
* src/conf/domain_conf.c (virDomainState): Use placeholder.
(virDomainSnapshotState): Extend mapping by one for use in snapshot.
(virDomainSnapshotDefParseString, virDomainSnapshotDefFormat):
Handle new state.
(virDomainObjSetState, virDomainStateReasonToString)
(virDomainStateReasonFromString): Avoid compiler warnings.
* tools/virsh.c (vshDomainState, vshDomainStateReasonToString):
Likewise.
* src/libvirt_private.syms (domain_conf.h): Export new functions.
* docs/schemas/domainsnapshot.rng: Tighten state definition.
* docs/formatsnapshot.html.in: Document it.
* tests/domainsnapshotxml2xmlout/disk_snapshot.xml: New test.
Easy enough to emulate even with older servers.
* tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Add
--halt flag.
(vshSnapshotCreate): Emulate halt when flag is unsupported.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
it.
It would technically be possible to have virsh compute the list
of descendants of a given snapshot, then delete those one at
a time. But it's complex, and not worth writing for a first
cut at implementing the new flags.
* tools/virsh.c (cmdSnapshotDelete): Add --children-only,
--metadata.
* tools/virsh.pod (snapshot-delete): Document them.
Similar to 'undefine --managed-save' (commit 83e849c1), we must
assume that the old API is unsafe; however, we cannot emulate
metadata-only deletion on older servers. Additionally, we have
the wrinkle that while virDomainUndefineFlags and managed save
cleanup were introduced in 0.9.4, it wasn't until 0.9.5 that
snapshots block undefine of a domain. Do the best we can given
the server we are talking to.
* tools/virsh.c (cmdUndefine): Add --snapshots-metadata flag.
* tools/virsh.pod (undefine, destroy, shutdown): Document effect
of snapshots.
Wire up the new snapshot creation flags in virsh. For convenience,
teach 'snapshot-current' how to make an existing snapshot become
current (can be used after upgrading to newer libvirt to recover
from the fact that the older libvirt lost track of the current
snapshot after a restart). The snapshot-create-as command is
intentionally not taught --redefine or --current, as this would
imply adding a lot of other options for everything else that can
appear in the <domainsnapshot> xml, but which is normally read-only.
Besides, redefining will usually be done on files created by
snapshot-dumpxml, rather than something built up by hand on the
command line. And now that we can redefine, we can edit.
* tools/virsh.c (cmdSnapshotCreate): Add --redefine, --current,
and --no-metadata.
(cmdSnapshotCreateAs): Add --no-metadata.
(cmdSnapshotCurrent): Add snapshotname to alter current snapshot.
(cmdSnapshotEdit): New command.
* tools/virsh.pod (snapshot-create, snapshot-create-as)
(snapshot-current, snapshot-edit): Document these.
The next patch will make snapshot creation more complex, so it's
better to avoid repetition of the complexity.
* tools/virsh.c (vshSnapshotCreate): New helper function.
(cmdSnapshotCreate, cmdSnapshotCreateAs): Use it.
New flag bits are worth exposing via virsh. In the case of
snapshot-list --roots, it's possible to emulate this even when
talking to an older server that lacks the bit; whereas
--metadata requires a newer server.
Although we don't use --security-info yet, the flag is already
documented for other dumpxml operations, and turning it on now
will make it useful when a future patch actually has to honor it.
* tools/virsh.c (cmdSnapshotDumpXML, cmdSnapshotCurrent): Add
--security-info.
(cmdSnapshotList): Add --roots, --metadata.
* tools/virsh.pod (snapshot-dumpxml, snapshot-current)
(snapshot-list): Document these.
Even though I recently added 'virsh snapshot-parent', doing it one
snapshot at a time is painful, so make it possible to expand the
snapshot-list table at once.
* tools/virsh.c (cmdSnapshotList): Add --parent.
* tools/virsh.pod (snapshot-list): Document it.
Pretty straight-forward exposure of new flags. For most commands,
we let the API reject mutually exclusive flags; but for save-image-edit,
we do the sanity check ourselves to avoid looping on flag failure if
the edit cycle is ever enhanced to allow the user to retry an edit
to fix up an xml validation error.
* tools/virsh.c (cmdManagedSave, cmdRestore, cmdSave)
(cmdSaveImageDefine, cmdSaveImageEdit): Add new flags.
* tools/virsh.pod (managedsave, restore, save, save-image-define)
(save-image-edit): Document them.
The 'virsh man' description of send-key was incomplete and used the
old style (literal 'optional name' instead of '[name]' metasyntax).
Meanwhile, none of the other virsh help texts include examples, so
I moved it out of virsh help and into the man page.
* tools/virsh.pod (send-key): Give better details.
* tools/virsh.c (info_send_key): Drop example from here.
Managed save was added in 0.8.0, virDomainCreateWithFlags in 0.8.2,
and FORCE_BOOT in 0.9.5. The virsh flag is more useful if we
emulate it for all older servers (note that if a hypervisor fails
the query for a managed save image, then it does not have one to
be removed, so the flag can be safely ignored).
* tools/virsh.c (cmdStart): Add emulation for new flag.
* tools/virsh.c: fix memory leak on cmdVolCreateAs function.
* Detected in valgrind run:
==4746==
==4746== 48 (40 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 26 of 52
==4746== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==4746== by 0x4C76E51: virAlloc (memory.c:101)
==4746== by 0x4CD9418: virGetStoragePool (datatypes.c:592)
==4746== by 0x4D21367: remoteStoragePoolLookupByName (remote_driver.c:4126)
==4746== by 0x4CE42B0: virStoragePoolLookupByName (libvirt.c:10232)
==4746== by 0x40C276: vshCommandOptPoolBy (virsh.c:13660)
==4746== by 0x40CA37: cmdVolCreateAs (virsh.c:8094)
==4746== by 0x412AF2: vshCommandRun (virsh.c:13770)
==4746== by 0x422F11: main (virsh.c:15127)
==4746==
==4746== 1,011 bytes in 1 blocks are definitely lost in loss record 45 of 52
==4746== at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==4746== by 0x4A06167: realloc (vg_replace_malloc.c:525)
==4746== by 0x4C76ECB: virReallocN (memory.c:161)
==4746== by 0x4C60319: virBufferGrow (buf.c:72)
==4746== by 0x4C606AA: virBufferAdd (buf.c:106)
==4746== by 0x40CB37: cmdVolCreateAs (virsh.c:8118)
==4746== by 0x412AF2: vshCommandRun (virsh.c:13770)
==4746== by 0x422F11: main (virsh.c:15127)
==4746==
==4746== LEAK SUMMARY:
==4746== definitely lost: 1,051 bytes in 2 blocks
==4746== indirectly lost: 8 bytes in 1 blocks
==4746== possibly lost: 0 bytes in 0 blocks
==4746== still reachable: 390,767 bytes in 1,373 blocks
==4746== suppressed: 0 bytes in 0 blocks
* How to reproduce?
% valgrind -v --leak-check=full virsh vol-create-as default foo.img 10M \
--allocation 0 --format qcow2 --backing-vol bar.img
Notes: bar.img doesn't exist.
Signed-off-by: Alex Jia <ajia@redhat.com>
virsh had some leftover 'int flags', and even an 'int flag'
declaration, compared to our preferred style of 'unsigned int flags'.
* tools/virsh.c (cmdUndefine, cmdSave, cmdSaveImageDumpxml)
(cmdSaveImageEdit, cmdManagedSave, cmdRestore, cmdDump)
(cmdVcpuPin, cmdSetvcpus, cmdSetmem, cmdSetmaxmem, cmdDumpXML)
(cmdDomXMLFromNative, cmdDomXMLToNative, doMigrate)
(cmdInterfaceEdit, cmdInterfaceDumpXML, cmdEdit): Match coding
style for flags.
(struct vshComdOptDef): Rename field member.
(vshCmddefOptParse, vshCmddefHelp): Adjust clients.
There have been several instances of people having problems with
a broken managed save file, and not aware that they could use
'virsh managedsave-remove dom' to fix things. Making it possible
to do this as part of starting a domain makes the same functionality
easier to find, and one less API call.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_START_FORCE_BOOT): New
flag.
* src/libvirt.c (virDomainCreateWithFlags): Document it.
* src/qemu/qemu_driver.c (qemuDomainObjStart): Alter signature.
(qemuAutostartDomain, qemuDomainStartWithFlags): Update callers.
* tools/virsh.c (cmdStart): Expose it in virsh.
* tools/virsh.pod (start): Document it.
Knowing whether 'virsh start' will resume a saved image or do
a fresh boot is useful enough to expose via 'virsh list'.
Also, translate the state column.
* tools/virsh.c (cmdList): add --managed-save flag
* tools/virsh.pod (list): Document it.
Based on a suggestion by Miklos Vajna.
Without this patch, invoking 'virsh >file 2>&1' results in
error messages appearing before normal output, even if they
occurred later in time than the normal output (since stderr
is unbuffered, but stdout waits until a full buffer).
* tools/virsh.c (print_job_progress, vshError): Flush between
stream transitions.
* tests/undefine: Test it.
Call me lazy, but:
virsh qemu-monitor-command dom --hmp info status
is nicer than:
virsh qemu-monitor-command dom --hmp 'info status'
* tools/virsh.c (cmdQemuMonitorCommand): Allow multiple arguments,
for convenience.
The public API documents that undefine may be used to transition a
running persistent domain into a transient one. Many drivers still
do not support this usage, but virsh shouldn't be getting in the
way of those that do support it.
This also drops a redundant conditional; vshCommandOptString
guaranteed that name was non-NULL.
* tools/virsh.c (cmdUndefine): Allow undefine on active domains;
the drivers may still reject it, but it is a valid API usage.
* tests/undefine (error): Fix the test to match.
Down the road, I want to add virDomainSnapshotGetParent, and use
the new API rather than xml scraping; but this virsh command can
be implemented even without the new API.
* tools/virsh.c (cmdSnapshotParent): New command.
* tools/virsh.pod (snapshot-parent): Document it.
Sometimes, full XML is too much; since most snapshot commands
operate on a snapshot name, there should be an easy way to get
at the current snapshot's name. For example:
virsh snapshot-revert dom `virsh snapshot-current dom --name`
* tools/virsh.c (cmdSnapshotCurrent): Add an option.
* tools/virsh.pod (snapshot-current): Document it.
This commands don't have a --pool option, so don't tell
vshCommandOptVolBy that there could be one. This made
vshCommandOptString for pooloptname fail and an "missing option"
error was reported.
Make pooloptname optional for vshCommandOptVolBy.
* tools/virsh.c: fix missing zero value judgement in cmdBlkiotune and correct
vshError information.
when weight is equal to 0, the cmdBlkiotune will not raise any error information
when judge weight value first time, and execute else branch to judge weight
value again, strncpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT, sizeof(temp->field))
will be not executed for ever. However, if and only if param->field is equal
to VIR_DOMAIN_BLKIO_WEIGHT, underlying qemuDomainSetBlkioParameters function
will check whether weight value is in range [100, 1000].
* how to reproduce?
% virsh blkiotune ${guestname} --weight 0
Signed-off-by: Alex Jia <ajia@redhat.com>
* tools/virsh.c: avoid memory leak in cmdVolPath.
* src/libvirt.c: Add doc for virStorageVolGetPath to tell one
must free() the returned path after use.
* how to reproduce?
% dd if=/dev/zero of=/var/lib/libvirt/images/foo.img count=1 bs=10M
% virsh pool-refresh default
% valgrind -v --leak-check=full virsh vol-path --vol \
/var/lib/libvirt/images/foo.img
* actual results:
Detected in valgrind run:
==16436== 32 bytes in 1 blocks are definitely lost in loss record 7 of 22
==16436== at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==16436== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so)
==16436== by 0x3DF8CD770D: xdr_remote_nonnull_string (remote_protocol.c:3
==16436== by 0x3DF8CD7EC8: xdr_remote_storage_vol_get_path_ret
% virsh pool-refresh default
% valgrind -v --leak-check=full virsh vol-path --vol \
/var/lib/libvirt/images/foo.img
Signed-off-by: Alex Jia <ajia@redhat.com>
