[libvirt] [RFC PATCH] iface-unbridge: Output in-use domain list instead of removing a in-used bridge forcibly

virsh iface-unbridge directly removes the bridge even though there are guest interfaces attach to. This patch outputs in-use domain list instead of removing bridge in this case. I knew that generally virsh-*.[ch] files are self contained, But for iterating all of domains, The patch broke this rule, So I sent it as RFC to see whether the patch makes sense or if there is a better way. Signed-off-by: Lin Ma <lma(a)suse.com&gt; --- tools/virsh-domain-monitor.c | 275 ------------------------------------------- tools/virsh-domain.c | 268 +++++++++++++++++++++++++++++++++++++++++ tools/virsh-domain.h | 13 ++ tools/virsh-interface.c | 99 ++++++++++++++++ 4 files changed, 380 insertions(+), 275 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 2af0d4f..b3d1b31 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1478,281 +1478,6 @@ static const vshCmdInfo info_list[] = { {.name = NULL} }; -/* compare domains, pack NULLed ones at the end*/ -static int -vshDomainSorter(const void *a, const void *b) -{ - virDomainPtr *da = (virDomainPtr *) a; - virDomainPtr *db = (virDomainPtr *) b; - unsigned int ida; - unsigned int idb; - unsigned int inactive = (unsigned int) -1; - - if (*da && !*db) - return -1; - - if (!*da) - return *db != NULL; - - ida = virDomainGetID(*da); - idb = virDomainGetID(*db); - - if (ida == inactive && idb == inactive) - return vshStrcasecmp(virDomainGetName(*da), virDomainGetName(*db)); - - if (ida != inactive && idb != inactive) { - if (ida > idb) - return 1; - else if (ida < idb) - return -1; - } - - if (ida != inactive) - return -1; - else - return 1; -} - -struct vshDomainList { - virDomainPtr *domains; - size_t ndomains; -}; -typedef struct vshDomainList *vshDomainListPtr; - -static void -vshDomainListFree(vshDomainListPtr domlist) -{ - size_t i; - - if (domlist && domlist->domains) { - for (i = 0; i < domlist->ndomains; i++) { - if (domlist->domains[i]) - virDomainFree(domlist->domains[i]); - } - VIR_FREE(domlist->domains); - } - VIR_FREE(domlist); -} - -static vshDomainListPtr -vshDomainListCollect(vshControl *ctl, unsigned int flags) -{ - vshDomainListPtr list = vshMalloc(ctl, sizeof(*list)); - size_t i; - int ret; - int *ids = NULL; - int nids = 0; - char **names = NULL; - int nnames = 0; - virDomainPtr dom; - bool success = false; - size_t deleted = 0; - int persistent; - int autostart; - int state; - int nsnap; - int mansave; - - /* try the list with flags support (0.9.13 and later) */ - if ((ret = virConnectListAllDomains(ctl->conn, &list->domains, - flags)) >= 0) { - list->ndomains = ret; - goto finished; - } - - /* check if the command is actually supported */ - if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { - vshResetLibvirtError(); - goto fallback; - } - - 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); - - vshResetLibvirtError(); - if ((ret = virConnectListAllDomains(ctl->conn, &list->domains, - newflags)) >= 0) { - list->ndomains = ret; - goto filter; - } - } - - /* there was an error during the first or second call */ - vshError(ctl, "%s", _("Failed to list domains")); - goto cleanup; - - - fallback: - /* fall back to old method (0.9.12 and older) */ - vshResetLibvirtError(); - - /* list active domains, if necessary */ - if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) || - VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE)) { - if ((nids = virConnectNumOfDomains(ctl->conn)) < 0) { - vshError(ctl, "%s", _("Failed to list active domains")); - goto cleanup; - } - - if (nids) { - ids = vshMalloc(ctl, sizeof(int) * nids); - - if ((nids = virConnectListDomains(ctl->conn, ids, nids)) < 0) { - vshError(ctl, "%s", _("Failed to list active domains")); - goto cleanup; - } - } - } - - if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) || - VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) { - if ((nnames = virConnectNumOfDefinedDomains(ctl->conn)) < 0) { - vshError(ctl, "%s", _("Failed to list inactive domains")); - goto cleanup; - } - - if (nnames) { - names = vshMalloc(ctl, sizeof(char *) * nnames); - - if ((nnames = virConnectListDefinedDomains(ctl->conn, names, - nnames)) < 0) { - vshError(ctl, "%s", _("Failed to list inactive domains")); - goto cleanup; - } - } - } - - list->domains = vshMalloc(ctl, sizeof(virDomainPtr) * (nids + nnames)); - list->ndomains = 0; - - /* get active domains */ - for (i = 0; i < nids; i++) { - if (!(dom = virDomainLookupByID(ctl->conn, ids[i]))) - continue; - list->domains[list->ndomains++] = dom; - } - - /* get inactive domains */ - for (i = 0; i < nnames; i++) { - if (!(dom = virDomainLookupByName(ctl->conn, names[i]))) - continue; - list->domains[list->ndomains++] = dom; - } - - /* truncate domains that weren't found */ - 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 (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT)) { - if ((persistent = virDomainIsPersistent(dom)) < 0) { - vshError(ctl, "%s", _("Failed to get domain persistence info")); - goto cleanup; - } - - if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT) && persistent) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) && !persistent))) - goto remove_entry; - } - - /* domain state filter */ - if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) { - if (virDomainGetState(dom, &state, NULL, 0) < 0) { - vshError(ctl, "%s", _("Failed to get domain state")); - goto cleanup; - } - - if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_RUNNING) && - state == VIR_DOMAIN_RUNNING) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_PAUSED) && - state == VIR_DOMAIN_PAUSED) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_SHUTOFF) && - state == VIR_DOMAIN_SHUTOFF) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_OTHER) && - (state != VIR_DOMAIN_RUNNING && - state != VIR_DOMAIN_PAUSED && - state != VIR_DOMAIN_SHUTOFF)))) - goto remove_entry; - } - - /* autostart filter */ - if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART)) { - if (virDomainGetAutostart(dom, &autostart) < 0) { - vshError(ctl, "%s", _("Failed to get domain autostart state")); - goto cleanup; - } - - if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) && autostart) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART) && !autostart))) - goto remove_entry; - } - - /* managed save filter */ - if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_MANAGEDSAVE)) { - if ((mansave = virDomainHasManagedSaveImage(dom, 0)) < 0) { - vshError(ctl, "%s", - _("Failed to check for managed save image")); - goto cleanup; - } - - if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) && mansave) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE) && !mansave))) - goto remove_entry; - } - - /* snapshot filter */ - if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)) { - if ((nsnap = virDomainSnapshotNum(dom, 0)) < 0) { - vshError(ctl, "%s", _("Failed to get snapshot count")); - goto cleanup; - } - if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) && nsnap > 0) || - (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) && nsnap == 0))) - goto remove_entry; - } - - /* the domain matched all filters, it may stay */ - continue; - - remove_entry: - /* the domain has to be removed as it failed one of the filters */ - virDomainFree(list->domains[i]); - list->domains[i] = NULL; - deleted++; - } - - finished: - /* sort the list */ - if (list->domains && list->ndomains) - qsort(list->domains, list->ndomains, sizeof(*list->domains), - vshDomainSorter); - - /* truncate the list if filter simulation deleted entries */ - if (deleted) - VIR_SHRINK_N(list->domains, list->ndomains, deleted); - - success = true; - - cleanup: - for (i = 0; nnames != -1 && i < nnames; i++) - VIR_FREE(names[i]); - - if (!success) { - vshDomainListFree(list); - list = NULL; - } - - VIR_FREE(names); - VIR_FREE(ids); - return list; -} - static const vshCmdOptDef opts_list[] = { {.name = "inactive", .type = VSH_OT_BOOL, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12550ff..b83e670 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -60,6 +60,274 @@ # define SA_SIGINFO 0 #endif +/* compare domains, pack NULLed ones at the end*/ +int +vshDomainSorter(const void *a, const void *b) +{ + virDomainPtr *da = (virDomainPtr *) a; + virDomainPtr *db = (virDomainPtr *) b; + unsigned int ida; + unsigned int idb; + unsigned int inactive = (unsigned int) -1; + + if (*da && !*db) + return -1; + + if (!*da) + return *db != NULL; + + ida = virDomainGetID(*da); + idb = virDomainGetID(*db); + + if (ida == inactive && idb == inactive) + return vshStrcasecmp(virDomainGetName(*da), virDomainGetName(*db)); + + if (ida != inactive && idb != inactive) { + if (ida > idb) + return 1; + else if (ida < idb) + return -1; + } + + if (ida != inactive) + return -1; + else + return 1; +} + +void +vshDomainListFree(vshDomainListPtr domlist) +{ + size_t i; + + if (domlist && domlist->domains) { + for (i = 0; i < domlist->ndomains; i++) { + if (domlist->domains[i]) + virDomainFree(domlist->domains[i]); + } + VIR_FREE(domlist->domains); + } + VIR_FREE(domlist); +} + +vshDomainListPtr +vshDomainListCollect(vshControl *ctl, unsigned int flags) +{ + vshDomainListPtr list = vshMalloc(ctl, sizeof(*list)); + size_t i; + int ret; + int *ids = NULL; + int nids = 0; + char **names = NULL; + int nnames = 0; + virDomainPtr dom; + bool success = false; + size_t deleted = 0; + int persistent; + int autostart; + int state; + int nsnap; + int mansave; + + /* try the list with flags support (0.9.13 and later) */ + if ((ret = virConnectListAllDomains(ctl->conn, &list->domains, + flags)) >= 0) { + list->ndomains = ret; + goto finished; + } + + /* check if the command is actually supported */ + if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) { + vshResetLibvirtError(); + goto fallback; + } + + 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); + + vshResetLibvirtError(); + if ((ret = virConnectListAllDomains(ctl->conn, &list->domains, + newflags)) >= 0) { + list->ndomains = ret; + goto filter; + } + } + + /* there was an error during the first or second call */ + vshError(ctl, "%s", _("Failed to list domains")); + goto cleanup; + + + fallback: + /* fall back to old method (0.9.12 and older) */ + vshResetLibvirtError(); + + /* list active domains, if necessary */ + if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) || + VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_ACTIVE)) { + if ((nids = virConnectNumOfDomains(ctl->conn)) < 0) { + vshError(ctl, "%s", _("Failed to list active domains")); + goto cleanup; + } + + if (nids) { + ids = vshMalloc(ctl, sizeof(int) * nids); + + if ((nids = virConnectListDomains(ctl->conn, ids, nids)) < 0) { + vshError(ctl, "%s", _("Failed to list active domains")); + goto cleanup; + } + } + } + + if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) || + VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) { + if ((nnames = virConnectNumOfDefinedDomains(ctl->conn)) < 0) { + vshError(ctl, "%s", _("Failed to list inactive domains")); + goto cleanup; + } + + if (nnames) { + names = vshMalloc(ctl, sizeof(char *) * nnames); + + if ((nnames = virConnectListDefinedDomains(ctl->conn, names, + nnames)) < 0) { + vshError(ctl, "%s", _("Failed to list inactive domains")); + goto cleanup; + } + } + } + + list->domains = vshMalloc(ctl, sizeof(virDomainPtr) * (nids + nnames)); + list->ndomains = 0; + + /* get active domains */ + for (i = 0; i < nids; i++) { + if (!(dom = virDomainLookupByID(ctl->conn, ids[i]))) + continue; + list->domains[list->ndomains++] = dom; + } + + /* get inactive domains */ + for (i = 0; i < nnames; i++) { + if (!(dom = virDomainLookupByName(ctl->conn, names[i]))) + continue; + list->domains[list->ndomains++] = dom; + } + + /* truncate domains that weren't found */ + 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 (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT)) { + if ((persistent = virDomainIsPersistent(dom)) < 0) { + vshError(ctl, "%s", _("Failed to get domain persistence info")); + goto cleanup; + } + + if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_PERSISTENT) && persistent) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_TRANSIENT) && !persistent))) + goto remove_entry; + } + + /* domain state filter */ + if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) { + if (virDomainGetState(dom, &state, NULL, 0) < 0) { + vshError(ctl, "%s", _("Failed to get domain state")); + goto cleanup; + } + + if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_RUNNING) && + state == VIR_DOMAIN_RUNNING) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_PAUSED) && + state == VIR_DOMAIN_PAUSED) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_SHUTOFF) && + state == VIR_DOMAIN_SHUTOFF) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_OTHER) && + (state != VIR_DOMAIN_RUNNING && + state != VIR_DOMAIN_PAUSED && + state != VIR_DOMAIN_SHUTOFF)))) + goto remove_entry; + } + + /* autostart filter */ + if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_AUTOSTART)) { + if (virDomainGetAutostart(dom, &autostart) < 0) { + vshError(ctl, "%s", _("Failed to get domain autostart state")); + goto cleanup; + } + + if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_AUTOSTART) && autostart) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART) && !autostart))) + goto remove_entry; + } + + /* managed save filter */ + if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_MANAGEDSAVE)) { + if ((mansave = virDomainHasManagedSaveImage(dom, 0)) < 0) { + vshError(ctl, "%s", + _("Failed to check for managed save image")); + goto cleanup; + } + + if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE) && mansave) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE) && !mansave))) + goto remove_entry; + } + + /* snapshot filter */ + if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_SNAPSHOT)) { + if ((nsnap = virDomainSnapshotNum(dom, 0)) < 0) { + vshError(ctl, "%s", _("Failed to get snapshot count")); + goto cleanup; + } + if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT) && nsnap > 0) || + (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT) && nsnap == 0))) + goto remove_entry; + } + + /* the domain matched all filters, it may stay */ + continue; + + remove_entry: + /* the domain has to be removed as it failed one of the filters */ + virDomainFree(list->domains[i]); + list->domains[i] = NULL; + deleted++; + } + + finished: + /* sort the list */ + if (list->domains && list->ndomains) + qsort(list->domains, list->ndomains, sizeof(*list->domains), + vshDomainSorter); + + /* truncate the list if filter simulation deleted entries */ + if (deleted) + VIR_SHRINK_N(list->domains, list->ndomains, deleted); + + success = true; + + cleanup: + for (i = 0; nnames != -1 && i < nnames; i++) + VIR_FREE(names[i]); + + if (!success) { + vshDomainListFree(list); + list = NULL; + } + + VIR_FREE(names); + VIR_FREE(ids); + return list; +} static virDomainPtr vshLookupDomainInternal(vshControl *ctl, diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h index f46538f..2e0d11a 100644 --- a/tools/virsh-domain.h +++ b/tools/virsh-domain.h @@ -41,4 +41,17 @@ virDomainPtr vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, extern const vshCmdDef domManagementCmds[]; +int vshDomainSorter(const void *a, const void *b); + +struct vshDomainList { + virDomainPtr *domains; + size_t ndomains; +}; + +typedef struct vshDomainList *vshDomainListPtr; + +void vshDomainListFree(vshDomainListPtr domlist); + +vshDomainListPtr vshDomainListCollect(vshControl *ctl, unsigned int flags); + #endif /* VIRSH_DOMAIN_H */ diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 6cacaf1..28b9e22 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -25,6 +25,7 @@ #include <config.h> #include "virsh-interface.h" +#include "virsh-domain.h" #include <libxml/parser.h> #include <libxml/tree.h> @@ -40,6 +41,91 @@ #include "virxml.h" #include "virstring.h" +static bool +vshBridgeInUse(vshControl *ctl, const char *br_name, char ***inuse_domnames) +{ + bool ret = false; + char **inuse_list = NULL; + vshDomainListPtr list = NULL; + virDomainPtr dom; + size_t i,j; + char *xml = NULL; + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + int ninterfaces; + int count = 0; + xmlNodePtr *interfaces = NULL; + + if (!(list = vshDomainListCollect(ctl, VIR_CONNECT_LIST_DOMAINS_ACTIVE))) { + vshError(ctl, "%s", _("Failed to get active domains list")); + exit(EXIT_FAILURE); + } + + if (list->ndomains <= 0) + goto cleanup; + + if (VIR_ALLOC_N(inuse_list, list->ndomains) < 0) { + vshError(ctl, "%s", _("Failed to allocate inuse_list")); + exit(EXIT_FAILURE); + } + + for (i = 0; i < list->ndomains; i++) { + dom = list->domains[i]; + xml = virDomainGetXMLDesc(dom, 0); + + if (!xml) { + vshError(ctl, "%s", _("Failed to get domain xml desc")); + exit(EXIT_FAILURE); + } + + xmldoc = virXMLParseStringCtxt(xml, _("(domain_definition)"), &ctxt); + + if (!xmldoc) { + vshError(ctl, "%s", _("Failed to parse string context")); + exit(EXIT_FAILURE); + } + + ninterfaces = virXPathNodeSet("./devices/interface", ctxt, &interfaces); + + if (ninterfaces < 0) { + vshError(ctl, "%s", _("Failed to count interfaces")); + exit(EXIT_FAILURE); + } + + for (j = 0; j < ninterfaces; j++) { + char *source = NULL; + char *target = NULL; + ctxt->node = interfaces[j]; + source = virXPathString("string(./source/@bridge" + "|./source/@dev" + "|./source/@network" + "|./source/@name)", ctxt); + target = virXPathString("string(./target/@dev)", ctxt); + + if (target && source && br_name && STREQ(source, br_name)) { + inuse_list[count++] = vshStrdup(ctl, virDomainGetName(dom)); + ret = true; + VIR_FREE(source); + VIR_FREE(target); + break; + } + + VIR_FREE(source); + VIR_FREE(target); + } + + VIR_FREE(interfaces); + VIR_FREE(xml); + xmlFreeDoc(xmldoc); + xmlXPathFreeContext(ctxt); + } + *inuse_domnames = inuse_list; + + cleanup: + vshDomainListFree(list); + return ret; +} + virInterfacePtr vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, const char *optname, @@ -1043,6 +1129,9 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) xmlDocPtr xml_doc = NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr top_node, if_node, cur; + int i; + size_t inuse_count; + char **inuse_list = NULL; /* Get a handle to the original device */ if (!(br_handle = vshCommandOptInterfaceBy(ctl, cmd, "bridge", @@ -1084,6 +1173,16 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(if_name); + if (vshBridgeInUse(ctl, br_name, &inuse_list)) { + inuse_count = virStringListLength(inuse_list); + vshPrint(ctl, "The bridge %s is in use by other guests:", br_name); + for (i = 0; i < inuse_count; i++) + vshPrint(ctl, "%s %s", i != 0 ? "," : "", inuse_list[i]); + vshPrint(ctl, "\n"); + virStringFreeList(inuse_list); + goto cleanup; + } + /* Find the <bridge> node under <interface>. */ if (virXPathNode("./bridge", ctxt) == NULL) { vshError(ctl, "%s", _("No bridge node in xml document")); -- 1.8.4