On Fri, May 04, 2018 at 17:25:30 +0800, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@xxxxxxxx>
> ---
> 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")
> @@ -13371,6 +13367,10 @@ static const vshCmdOptDef opts_event[] = {
> .type = VSH_OT_BOOL,
> .help = N_("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;
This is not necessary, VIR_APPEND_ELEMENT does that
> + 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;
This leaks the eventIdxList array. Also it would be preferrable just to
use one array for the case when events are enumerated and when all are
used ...
> + }
> + if (VIR_INSERT_ELEMENT(eventIdxList,
Use VIR_APPEND_ELEMENT instead.
> + 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]];
No ternaries, please use the same array, just fill it differently.
> + data[i].id = -1;
You can refactor the above to use VIR_APPEND_ELEMENT too so that the
same array can be used.
> }
> if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0)
> goto cleanup;
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
