<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 05/04/2018 06:44 PM, Peter Krempa
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<pre wrap="">On Fri, May 04, 2018 at 17:25:30 +0800, Lin Ma wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Signed-off-by: Lin Ma <a class="moz-txt-link-rfc2396E" href="mailto:lma@suse.com"><lma@suse.com></a>
---
tools/virsh-domain.c | 76 +++++++++++++++++++++++++++-------------------------
1 file changed, 39 insertions(+), 37 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 89aefbf86a..b35c9adaaa 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -13347,10 +13347,6 @@ static const vshCmdInfo info_event[] = {
static const vshCmdOptDef opts_event[] = {
VIRSH_COMMON_OPT_DOMAIN_OT_STRING(N_("filter by domain name, id or uuid"),
0),
- {.name = "event",
- .type = VSH_OT_STRING,
- .help = N_("which event type to wait for")
- },
{.name = "all",
.type = VSH_OT_BOOL,
.help = N_("wait for all events instead of just one type<a class="moz-txt-link-rfc2396E" href="mailto:%29@@-13371,6+13367,10@@staticconstvshCmdOptDefopts_event[]=%7B.type=VSH_OT_BOOL,.help=N_%28">")
@@ -13371,6 +13367,10 @@ static const vshCmdOptDef opts_event[] = {
.type = VSH_OT_BOOL,
.help = N_("</a>show timestamp for each printed event")
},
+ {.name = "event",
+ .type = VSH_OT_ARGV,
+ .help = N_("which event type to wait for")
+ },
{.name = NULL}
};
@@ -13382,12 +13382,14 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
int timeout = 0;
virshDomEventData *data = NULL;
size_t i;
- const char *eventName = NULL;
+ int *eventIdxList = NULL;
+ size_t nevents = 0;
int event = -1;
bool all = vshCommandOptBool(cmd, "all");
bool loop = vshCommandOptBool(cmd, "loop");
bool timestamp = vshCommandOptBool(cmd, "timestamp");
int count = 0;
+ const vshCmdOpt *opt = NULL;
virshControlPtr priv = ctl->privData;
if (vshCommandOptBool(cmd, "list")) {
@@ -13396,15 +13398,25 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
return true;
}
- if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0)
- return false;
- if (eventName) {
- for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
- if (STREQ(eventName, vshEventCallbacks[event].name))
- break;
- if (event == VIR_DOMAIN_EVENT_ID_LAST) {
- vshError(ctl, _("unknown event type %s"), eventName);
- return false;
+ if (vshCommandOptBool(cmd, "event")) {
+ if (VIR_ALLOC_N(eventIdxList, 1) < 0)
+ goto cleanup;
</pre>
</blockquote>
<pre wrap="">This is not necessary, VIR_APPEND_ELEMENT does that
</pre>
</blockquote>
<tt>will remove it.<br>
<br>
<br>
</tt>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<blockquote type="cite">
<pre wrap="">+ nevents = 1;
+ while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
+ if (opt->data) {
+ for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++)
+ if (STREQ(opt->data, vshEventCallbacks[event].name))
+ break;
+ if (event == VIR_DOMAIN_EVENT_ID_LAST) {
+ vshError(ctl, _("unknown event type %s"), opt->data);
+ return false;
</pre>
</blockquote>
<pre wrap="">This leaks the eventIdxList array. </pre>
</blockquote>
<tt>ok, will fix it through using 'goto cleanup' instead of 'return
false'<br>
<br>
<br>
</tt>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<pre wrap="">Also it would be preferrable just to
use one array for the case when events are enumerated and when all are
used ...
</pre>
</blockquote>
<tt>Do you mean that the code logical should not go '--all' but
'--event A' in the case 'virsh event --event A --all'?<br>
<br>
<br>
</tt>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<blockquote type="cite">
<pre wrap="">+ }
+ if (VIR_INSERT_ELEMENT(eventIdxList,
</pre>
</blockquote>
<pre wrap="">Use VIR_APPEND_ELEMENT instead.</pre>
</blockquote>
<tt>will do.<br>
<br>
<br>
</tt>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ nevents - 1,
+ nevents,
+ event) < 0)
+ goto cleanup;
+ }
}
} else if (!all) {
vshError(ctl, "%s",
@@ -13412,26 +13424,15 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
return false;
}
- if (all) {
- if (VIR_ALLOC_N(data, VIR_DOMAIN_EVENT_ID_LAST) < 0)
- goto cleanup;
- for (i = 0; i < VIR_DOMAIN_EVENT_ID_LAST; i++) {
- data[i].ctl = ctl;
- data[i].loop = loop;
- data[i].count = &count;
- data[i].timestamp = timestamp;
- data[i].cb = &vshEventCallbacks[i];
- data[i].id = -1;
- }
- } else {
- if (VIR_ALLOC_N(data, 1) < 0)
- goto cleanup;
- data[0].ctl = ctl;
- data[0].loop = vshCommandOptBool(cmd, "loop");
- data[0].count = &count;
- data[0].timestamp = timestamp;
- data[0].cb = &vshEventCallbacks[event];
- data[0].id = -1;
+ if (VIR_ALLOC_N(data, all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1) < 0)
+ goto cleanup;
+ for (i = 0; i < (all ? VIR_DOMAIN_EVENT_ID_LAST : nevents - 1); i++) {
+ data[i].ctl = ctl;
+ data[i].loop = loop;
+ data[i].count = &count;
+ data[i].timestamp = timestamp;
+ data[i].cb = &vshEventCallbacks[all ? i : eventIdxList[i]];
</pre>
</blockquote>
<pre wrap="">No ternaries, </pre>
</blockquote>
<tt>ok<br>
<br>
<br>
</tt>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<pre wrap="">please use the same array, just fill it differently.
</pre>
</blockquote>
<tt>do you mean that<br>
whatever in 'all' or 'not all', I should fill '</tt><tt><tt>eventIdxList'
differently & </tt>use 'eventIdxList' in just one for loop?<br>
if so, that means:<br>
in case '--all': the value of array </tt><tt><tt>eventIdxList
will be from 0 through VIR_DOMAIN_EVENT_ID_LAST - 1.<br>
in case '--event' or '--event and --all': </tt></tt><tt><tt><tt>the
value of array </tt><tt><tt>eventIdxList will be
corresponding event id(s).<br>
not sure what I understand is right.<br>
<br>
<br>
</tt></tt> </tt></tt>
<blockquote type="cite"
cite="mid:20180504104459.GE38016@angien.pipo.sk">
<blockquote type="cite">
<pre wrap="">+ data[i].id = -1;
</pre>
</blockquote>
<pre wrap="">You can refactor the above to use VIR_APPEND_ELEMENT too so that the
same array can be used.
</pre>
</blockquote>
<tt> Sorry, I didn't get the point. which one should be refactored
to use VIR_APPEND_ELEMENT as well? 'eventIdxList' for case
'--all'? <br>
<br>
<br>
<br>
Thanks,<br>
Lin<br>
</tt>
</body>
</html>