Re: [libvirt] [PATCH 2/2] virsh: Pass the corect live/config xml to virshDomainDetachInterface.
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(a)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