[libvirt] [PATCH v4 2/3] qemu: Implement extended loader and nvram

Michal Privoznik mprivozn at redhat.com
Mon Sep 1 13:13:31 UTC 2014


On 22.08.2014 18:48, Eric Blake wrote:
> On 08/21/2014 02:50 AM, Michal Privoznik wrote:
>> QEMU now supports UEFI with the following command line:
>>
>>    -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
>>    -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \
>>
>> where the first line reflects <loader> and the second one <nvram>.
>> Moreover, these two lines obsoletes the -bios argument.
>
> s/obsoletes/obsolete/
>
>>
>> Note that UEFI is unusable without ACPI. This is handled properly now.
>> Among with this extension, the variable file is expected to be
>> writable and hence we need security drivers to label it.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>
>> +    case VIR_DOMAIN_LOADER_TYPE_PFLASH:
>> +        /* UEFI is supported only for x86_64 currently */
>> +        if (def->os.arch != VIR_ARCH_X86_64) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                           _("pflash is not supported for %s guest achitecture"),
>
> s/achitecture/architecture/
>
>
>> +
>> +        if (loader->readonly) {
>> +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                               _("this qemu doesn't support passing "
>> +                                 "readonly attribute"));
>> +                goto cleanup;
>> +            }
>> +
>> +            virBufferAsprintf(&buf, ",readonly=%s",
>> +                              virTristateSwitchTypeToString(loader->readonly));
>> +        }
>> +
>> +        virCommandAddArg(cmd, "-drive");
>> +        virCommandAddArgBuffer(cmd, &buf);
>> +
>> +        if (loader->nvram) {
>> +            virBufferFreeAndReset(&buf);
>> +            virBufferAsprintf(&buf,
>> +                              "file=%s,if=pflash,format=raw,unit=%d",
>> +                              loader->nvram, unit);
>
> Hmm.  Here, it looks like readonly='on' is supported ONLY for
> type='pflash', and not for type='rom'.  In that case, I'd write the .rng
> of patch 1 as (rough draft):
>
> <element name='loader'>
>    <choice>
>      <group> <!-- bios, default loader type -->
>        <optional>
>          <attribute name='type'>
>            <value>rom</value>
>          </attribute>
>        </optional>
>      </group>
>      <group> <!-- pflash, for OVMF use -->
>        <attribute name='type'>
>          <value>pflash</value>
>        </attribute>
>        <optional>
>          <attribute name='readonly'...>
>        </optional>
>        <optional>
>          nvram stuff...
>        </optional>
>      </group>
>    </choice>
>    <ref name='absFileName'>
> </element>

While this can be correct I think having wider XML schema then the code 
is just okay. Keeping the schema readable wins over tightening all the 
narrow cases for me.

Michal




More information about the libvir-list mailing list