On 07/23/2018 04:23 PM, Han Han wrote:
> On Mon, Jul 23, 2018 at 6:21 PM, Michal Prívozník <mprivozn@xxxxxxxxxx>
> 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 <hhan@xxxxxxxxxx>
>>> ---
>>> 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?
Well, currently it reports an error if alias is specified and does not
fill all the requirements. Remember, having "ua-" prefix is just one of
the requirements.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
