[libvirt] [PATCH] virsh: Separate 'create'/'modify' message for secret-define

[Li, Yang]

> On Fri, Apr 18, 2014 at 12:20:26 +0200, Martin Kletzander wrote:
> > On Fri, Apr 18, 2014 at 04:27:10AM -0400, Li Yang wrote:
> > >The current message of secret-define always be:
> > >Secret 09a9736f-eedb-449c-9983-80d0ab67393f created
> >
> > >even you just modify the secret, perhaps this may puzzle
> > >uses. Now this patch make the modify action output message
> > >like this:
> > >Secret f2d1bafc-ac58-4a47-93e4-47723686fef5 modified
> > >
> > >Signed-off-by: Li Yang <liyang.fnst at cn.fujitsu.com>
> > >---
> > > tools/virsh-secret.c |   16 +++++++++++++++-
> > > 1 files changed, 15 insertions(+), 1 deletions(-)
> > >
> > >diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c
> > >index 10d5db3..d1cbe04 100644
> > >--- a/tools/virsh-secret.c
> > >+++ b/tools/virsh-secret.c
> > >@@ -39,6 +39,7 @@
> > > #include "virutil.h"
> > > #include "virxml.h"
> > > #include "conf/secret_conf.h"
> > >+#include "viruuid.h"
> > >
> > > static virSecretPtr
> > > vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char **name)
> > >@@ -96,6 +97,7 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
> > >     char *buffer;
> > >     virSecretPtr res;
> > >     char uuid[VIR_UUID_STRING_BUFLEN];
> > >+    virSecretDefPtr new_attrs;
> > >     bool ret = false;
> > >
> > >     if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
> > >@@ -104,6 +106,12 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
> > >     if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
> > >         return false;
> > >
> > >+    new_attrs = virSecretDefParseString(buffer);
> > >+    if (new_attrs == NULL){
> > >+        VIR_FREE(buffer);
> > >+        return false;
> > >+    }
> > >+
> > >     if (!(res = virSecretDefineXML(ctl->conn, buffer, 0))) {
> > >         vshError(ctl, _("Failed to set attributes from %s"), from);
> > >         goto cleanup;
> > >@@ -114,10 +122,16 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd)
> > >         goto cleanup;
> > >     }
> > >
> > >-    vshPrint(ctl, _("Secret %s created\n"), uuid);
> > >+    char uuidstr[VIR_UUID_STRING_BUFLEN];
> > >+    virUUIDFormat(new_attrs->uuid, uuidstr);
> > >+    if (memcmp(uuid, uuidstr, VIR_UUID_BUFLEN) == 0)
> > >+        vshPrint(ctl, _("Secret %s modified\n"), uuid);
> > >+    else
> > >+        vshPrint(ctl, _("Secret %s created\n"), uuid);
> > >     ret = true;
> > 
> > You'll still print 'modified' if the new created secret has an uuid in
> > the file already, plus it does more parsing, etc.  Wouldn't it be
> > easier to just do s/created/defined/ ?
>
> Also you compare just the first VIR_UUID_BUFLEN characters in UUID
> string. I agree with Martin.
>
> Jirka

Oh, yes, you are right, I didn't notice that. I will try another way to cover this situation.
If I cannot find an easy way to solve this  problem, I think we'd better leave it at that,
Modify "created" to "defined" seems not enough useful for uses...
After all, secret-undefine command's output is "Secret *** deleted", it's correspond
to secret-define command's output "Secret *** created".