[virt-tools-list] [PATCH 0/2] virtinst: Pass SMBios information to guest

[Charles Arnold]

>>> On 7/18/2016 at 12:54 PM, Cole Robinson <crobinso at redhat.com> wrote: 
> On 07/14/2016 10:06 AM, Charles Arnold wrote:
>> This patchset introduces a command line option to pass smbios information to
>> the guest. It adds an smbios mode sub-element to the guest's os element. The
>> mode attribute must be specified and is either "emulate" (let the hypervisor
>> generate all values), "host" (copy all of Block 0 and Block 1, except for 
> the
>> UUID, from the host's SMBIOS values), or "sysinfo" (use the values in the
>> sysinfo element). If not specified, the hypervisor default is used.
>> 
>> If sysinfo is specified in the smbios mode field then an additional sysinfo
>> element is added to the guest defining the SMBios information. The sysinfo
>> element has a mandatory attribute type of "smbios". The smbios information
>> comes as three sub-types, "bios", "system", and "baseBoard" where each of
>> these types contains sub-elements .
>> 
>> Incorrectly supplied entries for the bios, system or baseBoard blocks will 
> be
>> ignored without error. Other than uuid validation and date format checking, 
> all
>> values are passed as strings to the hypervisor driver.
>> 
>> Examples:
>>     --sysinfo emulate
>>     --sysinfo host
>>     --sysinfo type=0,vendor=Vendor_Inc.,version=1.2.3-abc,...
>>     --sysinfo type=1,manufacturer=System_Corp.,product=codename,...
>>     --sysinfo type=2,manufacturer=Baseboard_Corp.,product=codename,...
>> 
>> 
> 
> Thanks for reworking the patches. I see now that you were trying to mirror 
> the
> qemu command line -smbios format. Were you just following that as an example,
> or do you have a particularly compelling reason for it?

Just following it as an example. 

> 
> virt-install cli naming should map to libvirt values as much as possible,
> (minus the '<entry>' naming which is completely redundant) maybe some day
> libvirt will support a non-qemu <sysinfo>, or a non-smbios <sysinfo>, or the
> user doesn't know that the syntax is for matching with qemu, etc, so we
> shouldn't be tied to qemu syntax.

Agreed. Libvirt's 'virsh sysinfo' command could someday be enhanced to do
what I'm trying to do with virt-install. If the work gets done it may also follow
some entirely different syntax.

> 
> I pushed some patches upstream that make XML properties like this work
> correctly when setting the value:
> 
>   bios_vendor = XMLProperty("./bios/entry[@name='vendor']")
> 
> So I think SYSInfo should be altered to explicitly track bios_* , system_*,
> and baseBoard_* properties, and the --sysinfo command line option should have
> bios.*, system.*, and baseboard.* properties to match. That way '--sysinfo
> help' introspection works as expected too.

Ok, I'll work on that.

> 
> _If_ providing an option for the qemu syntax is important to you we can 
> still
> support it, but it needs to be explicit. Maybe something like --sysinfo
> qemu-cli='type=1,Manufacturer=X,...'. But it should be an additive thing, not
> the main way to interact

I'm not sold on any particular syntax for providing the information. Adding more
ways to provide the same thing may just unnecessarily complicate the usage.

Thanks for taking time to do the review,

Charles

> 
> Thanks,
> Cole