On 05/02/2016 09:59 AM, Nitesh Konkar wrote:
cmdDetachInterface function checks for live config flags and then passes the live/config domain xml to virshDomainDetachInterface accordingly.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- tools/virsh-domain.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d9fde4d..5cfd6a3 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11302,7 +11302,7 @@ static bool cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - char *doc = NULL; + char *doc_live = NULL, *doc_config = NULL; bool functionReturn = false; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; bool current = vshCommandOptBool(cmd, "current"); @@ -11317,8 +11317,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
Hmm, I'm having trouble following the logic here with the flag manipulation. I think there's an error here with --persistent handling at least. I suggest to drop the 'flags' variable entirely. So...
if (config || persistent) flags |= VIR_DOMAIN_AFFECT_CONFIG;
Here I'd change to bool affect_config; ... affect_config = (config || persistent);
- if (live) - flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -11327,15 +11325,23 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) virDomainIsActive(dom) == 1) flags |= VIR_DOMAIN_AFFECT_LIVE;
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) - doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE); - else - doc = virDomainGetXMLDesc(dom, 0); + if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
This will just be (affect_config)
+ if (!(doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + if (!(functionReturn = virshDomainDetachInterface(doc_config, flags, dom, ctl, cmd))) + goto cleanup; + }
- if (!doc) - goto cleanup; - else - functionReturn = virshDomainDetachInterface(doc, flags, dom, ctl, cmd); + flags = 0; + + if (live || (!live && !config)) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
This will be: bool affect_live; ... affect_live = (live || (persistent && virDomainIsActive(dom) == 1)); if (affect_live || !affect_config) int flags = 0; if (affect_live) flags |= VIR_DOMAIN_AFFECT_LIVE; ... The last bit with the 'int flags' ensures we still use flags=0 and old style virDomainDetachDevice() if the user doesn't specify any command line flags. We want to preserve that behavior for compat with older libvirt - Cole