<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 23, 2018 at 6:21 PM, Michal Prívozník <span dir="ltr"><<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 07/15/2018 12:08 PM, Han Han wrote:<br>
> Add --alias to support custom disk alias in virsh attach-disk.<br>
> Report error if custom alias doesn't start with 'ua-'.<br>
> <br>
> Signed-off-by: Han Han <<a href="mailto:hhan@redhat.com">hhan@redhat.com</a>><br>
> ---<br>
>  tools/virsh-domain.c | 15 ++++++++++++++-<br>
>  tools/virsh.pod      |  3 ++-<br>
>  2 files changed, 16 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c<br>
> index 8adec1d9b1..467417852e 100644<br>
> --- a/tools/virsh-domain.c<br>
> +++ b/tools/virsh-domain.c<br>
> @@ -290,6 +290,10 @@ static const vshCmdOptDef opts_attach_disk[] = {<br>
>       .type = VSH_OT_STRING,<br>
>       .help = N_("wwn of disk device")<br>
>      },<br>
> +    {.name = "alias",<br>
> +     .type = VSH_OT_STRING,<br>
> +     .help = N_("custom alias name of disk device")<br>
> +    },<br>
>      {.name = "rawio",<br>
>       .type = VSH_OT_BOOL,<br>
>       .help = N_("needs rawio capability")<br>
> @@ -574,7 +578,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)<br>
>                  *subdriver = NULL, *type = NULL, *mode = NULL,<br>
>                  *iothread = NULL, *cache = NULL, *io = NULL,<br>
>                  *serial = NULL, *straddr = NULL, *wwn = NULL,<br>
> -                *targetbus = NULL;<br>
> +                *targetbus = NULL, *alias = NULL;<br>
>      struct DiskAddress diskAddr;<br>
>      bool isFile = false, functionReturn = false;<br>
>      int ret;<br>
> @@ -611,6 +615,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)<br>
>          vshCommandOptStringReq(ctl, cmd, "wwn", &wwn) < 0 ||<br>
>          vshCommandOptStringReq(ctl, cmd, "address", &straddr) < 0 ||<br>
>          vshCommandOptStringReq(ctl, cmd, "targetbus", &targetbus) < 0 ||<br>
> +        vshCommandOptStringReq(ctl, cmd, "alias", &alias) < 0 ||<br>
>          vshCommandOptStringReq(ctl, cmd, "sourcetype", &stype) < 0)<br>
>          goto cleanup;<br>
>  <br>
> @@ -680,6 +685,14 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)<br>
>      if (serial)<br>
>          virBufferAsprintf(&buf, "<serial>%s</serial>\n", serial);<br>
>  <br>
> +    if (alias) {<br>
> +        if (!STRPREFIX(alias, "ua-")) {<br>
> +            vshError(ctl, _("Custom alias name should start with ua-"));<br>
> +            goto cleanup;<br>
> +        }<br>
<br>
</div></div>I don't think this belongs here. I'd let libvirt report an error. The<br>
reason for that is to have checks in one place rather than scattered<br>
around the code. For instance, imagine that one day we lift the<br>
restriction and let users define alias in a free form. Using this<br>
version of virsh (and connecting to new libvirtd) they will be unable to<br>
do so.<br>
Or the other way round - we allow only certain characters to be in user<br>
alias. You are not checking them here - you're relying on libvirtd doing so.<br>
<br></blockquote><div>It is reasonable that libvirtd check the alias name. As I know, currently libvirtd</div><div>will ignore the customized alias not starting with 'ua-'. Will we keep ignoring</div><div>it or report an error in libvirtd?<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The rest looks okay.<br>
<span class="HOEnZb"><font color="#888888"><br>
Michal<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr">Best regards,</div><div dir="ltr">-----------------------------------<br></div><div dir="ltr">Han Han<br>Quality Engineer<br>Redhat.<br><br>Email: <a href="mailto:hhan@redhat.com" target="_blank">hhan@redhat.com</a><br>Phone: +861065339333<br></div></div></div></div></div></div>
</div></div>