[libvirt] [PATCH] qemu: Allow the user to specify vendor and product for disk

[Martin Kletzander]

On 11/08/2012 02:52 AM, Dave Allan wrote:
> On Thu, Nov 08, 2012 at 09:28:06AM +0800, Osier Yang wrote:
>> On 2012年11月08日 05:04, Dave Allan wrote:
>>> On Wed, Nov 07, 2012 at 11:24:43PM +0800, Osier Yang wrote:
>>>> On 2012年11月05日 21:34, Martin Kletzander wrote:
>>>>> On 11/05/2012 08:04 AM, Osier Yang wrote:
>>>>>> QEMU supports to set vendor and product strings for disk since
>>>>>> 1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch
>>>>>> exposes it with new XML elements<vendor>   and<product>   of disk
>>>>>> device.
>>>>>> ---
>>>>>>  docs/formatdomain.html.in                          |   10 +++++
>>>>>>  docs/schemas/domaincommon.rng                      |   10 +++++
>>>>>>  src/conf/domain_conf.c                             |   30 ++++++++++++++++
>>>>>>  src/conf/domain_conf.h                             |    2 +
>>>>>>  src/qemu/qemu_command.c                            |   29 ++++++++++++++++
>>>>>>  .../qemuxml2argv-disk-scsi-disk-vpd.args           |   13 +++++++
>>>>>>  .../qemuxml2argv-disk-scsi-disk-vpd.xml            |   36 ++++++++++++++++++++
>>>>>>  tests/qemuxml2argvtest.c                           |    4 ++
>>>>>>  8 files changed, 134 insertions(+), 0 deletions(-)
>>>>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
>>>>>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml
>>>>>>
>>>>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>>>>> index c8da33d..cc9e871 100644
>>>>>> --- a/docs/formatdomain.html.in
>>>>>> +++ b/docs/formatdomain.html.in
>>>>>> @@ -1657,6 +1657,16 @@
>>>>>>          of 16 hexadecimal digits.
>>>>>>          <span class='since'>Since 0.10.1</span>
>>>>>>        </dd>
>>>>>> +<dt><code>vendor</code></dt>
>>>>>> +<dd>If present, this element specifies the vendor of a virtual hard
>>>>>> +        disk or CD-ROM device. It's a not more than 8 bytes alphanumeric string.
>>>>>
>>>>> Last sentence doesn't make sense, I suggest changing it to either: "It
>>>>> can't be longer than 8 alphanumeric characters." or simply "Maximum 8
>>>>> alphanumeric characters."
>>>>
>>>> Okay, honestly, I wasn't comfortable with the sentence, but can't
>>>> get a better one at that time, :-) I will change your advise a bit:
>>>>
>>>> It must not be longer than 8 alphanumeric characters.
>>>
>>> Not to be pedantic, but what happens if you hand it doublebyte
>>> characters?
>>
>> Ah, good question, though QEMU will truncate the string to 8 bytes
>> anyway, should we do some translation in libvirt? the problem is
>> how to map the doublebytes vendors/product to single bytes ones,
>> is there some public specification available? /me starts to think
>> if it's snake leg (or breaking fly on the wheel) to do the checking
>> if it's too heavy.
> 
> Mostly I was curious about how the code handled doublebyte characters,
> but now that I think about it, the SCSI spec mandates 8 bytes of
> ASCII[1], but it sounds like qemu is already enforcing that, so I
> think it's fine just to let it be freeform and note the spec's
> requirement in the documentation.
> 

I'd say if QEMU starts (and truncates or whatever) with invalid vendor
name, there's no problem for us to leave the responsibility on the user,
but OTOH when QEMU won't like what it's doing and will eventually
fix/change that, our machines may not start.  So if we know the
limitation (and it is very easy one), why don't we just check (and
mention it in the docs) that we accept maximum 8 (16) alphanumeric
characters, checking just [A-Za-z0-9] instead of doing some 'isalnum()'
magic.  Check is easy, QEMU will be always satisfied with the option, I
see it as a win-win.


Martin