On 06/20/2012 07:33 AM, Peter Krempa wrote:
This patch makes use of the newly added api
virConnectListAllDomains()
to list domains in virsh.
Virsh now represents lists of domains using an internal structure
vshDomainList. This structure contains the virDomainPtr list as provided
by virConnectListAllDomains() and the count of domains in the list.
For backwards compatiblity, the function vshDomainListCollect was added
s/compatiblity/compatibility/
that tries to enumerate the domains using the new API and if the API
is
not supported falls back to the older approach with the two list
functions. The helper function also simulates filtering by all
currently supported flags added with virConnectListAllDomains().
This patch also cleans up the "list" command handler to use the new
helpers and adds new command line flags to make use of filtering.
---
Diff to v3:
-Fixed compacting of domain list in fallback filter
-removed redundant filter for active state
-split out re-indentation and rename of namesorter to a separate patch
-renamed functions
-renamed flags
-tweaked docs (typos/grammar)
+static void
+vshDomainListFree(vshDomainListPtr domlist)
+{
+ int i;
+
+ if (!domlist || !domlist->domains)
+ return;
Memory leak if domlist && !domlist->domains.
+
+ if (last_error && last_error->code == VIR_ERR_INVALID_ARG) {
+ /* try the new API again but mask non-guaranteed flags */
+ unsigned int newflags = flags & (VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE);
Might be easier to use VIR_CONNECT_LIST_FILTERS_ACTIVE from virdomainlist.h.
+ /* fall back to old method (0.9.12 and older) */
+ virResetLastError();
+
+ /* list active domains, if necessary */
+ if (!MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE) ||
Again, using ...LIST_FILTERS_ACTIVE might be easier.
+
+ /* truncate domanis that weren't found */
s/domanis/domains/
+ deleted = (nids + nnames) - list->ndomains;
+
+filter:
+ /* filter list the list if the list was acquired by fallback means */
+ for (i = 0; i < list->ndomains; i++) {
+ dom = list->domains[i];
+
+ /* persistence filter */
+ if (MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_TRANSIENT)) {
VIR_CONNECT_LIST_FILTERS_PERSISTENT (and so forth, I'll quit pointing it
out)
+ /* autostart filter */
+ if (MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART |
+ VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART)) {
+ if (virDomainGetAutostart(dom, &autostart) < 0) {
+ vshError(ctl, "%s", _("Failed to get domain autostart
state"));
+ goto cleanup;
+ }
Here, I think it is safe to assume that failure of
virDomainGetAutostart() implies that we can treat the domain like it
does not support autostart (that is, match DOMAINS_NO_AUTOSTART),
instead of erroring out. But up to you; I'm also okay with the error,
because in that case, you can avoid the error by not passing in the flag
to virsh in the first place.
+
+ if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) && autostart) ||
+ (MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART) &&
!autostart)))
+ goto remove_entry;
+ }
+
+ /* managed save filter */
+ if (MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE |
+ VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE)) {
+ if ((mansave = virDomainHasManagedSaveImage(dom, 0)) < 0) {
+ vshError(ctl, "%s",
+ _("Failed to check for managed save image"));
+ goto cleanup;
+ }
Same story - if the API fails, you can probably safely assume there is
no managed save image.
+
+ if (!((MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) && mansave) ||
+ (MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE) &&
!mansave)))
+ goto remove_entry;
+ }
+
+ /* snapshot filter */
+ if (MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT |
+ VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT)) {
+ if ((nsnap = virDomainSnapshotNum(dom, 0)) < 0) {
+ vshError(ctl, "%s", _("Failed to get snapshot
count"));
+ goto cleanup;
+ }
Likewise, if the API fails, you can assume no snapshots.
+
+To filter the list of domains present on the hypervisor you may specify one or
+more of filtering flags supported by the B<list> command. These flags are
s/more of/more/
ACK with typo fixes and memory leak plugged. I'm okay if you push
without posting a v5, now that the fixes are down to a manageable amount.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org