On 04/28/2016 05:11 AM, Nitesh Konkar wrote:
virshDomainDetachInterface handles virsh interface detach from the specified live/config domain xml.
Signed-off-by: Nitesh Konkar <nitkon12@linux.vnet.ibm.com> --- tools/virsh-domain.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 36d0353..7cb042b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11118,6 +11118,105 @@ static const vshCmdOptDef opts_detach_interface[] = { };
I meant that this patch should also be removing the functionality cmdDetachInterface at the same time. So patch #1: break out the old functionality to virshDomainDetachInterface, and use it in cmdDetachInterface. The change should be a functional no-op Then patch #2 changes the behavior. The point is that you isolate the functional changes in the smallest patch possible, since that is the harder stuff to review, but a smaller patch will make it easier to review.
static bool + virshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, vshControl *ctl, const vshCmd *cmd) +{
This line is too long. Split it like: static bool virshDomainDetachInterface(char *doc, unsigned int flags, ...
+ xmlDocPtr xml = NULL; + xmlXPathObjectPtr obj = NULL; + xmlXPathContextPtr ctxt = NULL; + xmlNodePtr cur = NULL, matchNode = NULL; + const char *mac = NULL, *type = NULL; + char *detach_xml = NULL, buf[64]; + bool current = vshCommandOptBool(cmd, "current"); + int diff_mac,ret;
The style is wrong here, I see you fixed it in patch 2, but the change should have been in this patch. Make sure every patch is syntax-check clean Please fix those issues and repost Thanks, Cole