[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]