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