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

2014-09-01 Thread Michal Privoznik 

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  and the second one .
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 
---



+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):


   
  
   
 
   rom
 
   
 
  
   
 pflash
   
   
 
   
   
 nvram stuff...
   
 
   
   



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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-08-22 Thread Eric Blake 
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  and the second one .
> 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 
> ---

> +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):


  
 
  

  rom

  

 
  
pflash
  
  

  
  
nvram stuff...
  

  
  


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-08-22 Thread Laszlo Ersek 
On 08/21/14 10:50, 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  and the second one .
> Moreover, these two lines obsoletes the -bios argument.
> 
> 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 
> ---
>  src/qemu/qemu_command.c| 94 
> +-
>  src/security/security_dac.c|  8 ++
>  src/security/security_selinux.c|  8 ++
>  .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  | 10 +++
>  tests/qemuxml2argvtest.c   |  2 +
>  5 files changed, 118 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args

Acked-by: Laszlo Ersek 

(Please append this line to the commit message in any further reposts.)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-08-21 Thread Michal Privoznik 
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  and the second one .
Moreover, these two lines obsoletes the -bios argument.

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 
---
 src/qemu/qemu_command.c| 94 +-
 src/security/security_dac.c|  8 ++
 src/security/security_selinux.c|  8 ++
 .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args  | 10 +++
 tests/qemuxml2argvtest.c   |  2 +
 5 files changed, 118 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d03c4d5..2a6eee5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7282,6 +7282,94 @@ qemuBuildChrDeviceCommandLine(virCommandPtr cmd,
 return 0;
 }
 
+static int
+qemuBuilDomainLoaderCommandLine(virCommandPtr cmd,
+virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps)
+{
+int ret = -1;
+virDomainLoaderDefPtr loader = def->os.loader;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int unit = 0;
+
+if (!loader)
+return 0;
+
+switch ((virDomainLoader) loader->type) {
+case VIR_DOMAIN_LOADER_TYPE_ROM:
+virCommandAddArg(cmd, "-bios");
+virCommandAddArg(cmd, loader->path);
+break;
+
+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"),
+   virArchToString(def->os.arch));
+goto cleanup;
+}
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this qemu doesn't support -drive"));
+goto cleanup;
+}
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this qemu doesn't support passing "
+ "drive format"));
+goto cleanup;
+}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI) &&
+def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("ACPI must be enabled in order to use UEFI"));
+goto cleanup;
+}
+
+virBufferAsprintf(&buf,
+  "file=%s,if=pflash,format=raw,unit=%d",
+  loader->path, unit);
+unit++;
+
+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);
+
+virCommandAddArg(cmd, "-drive");
+virCommandAddArgBuffer(cmd, &buf);
+}
+break;
+
+case VIR_DOMAIN_LOADER_TYPE_LAST:
+/* nada */
+break;
+}
+
+ret = 0;
+ cleanup:
+virBufferFreeAndReset(&buf);
+return ret;
+}
+
 qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
 .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
 };
@@ -7437,10 +7525,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, "-enable-nesting");
 }
 
-if (def->os.loader) {
-virCommandAddArg(cmd, "-bios");
-virCommandAddArg(cmd, def->os.loader->path);
-}
+if (qemuBuilDomainLoaderCommandLine(cmd, def, qemuCaps) < 0)
+goto error;
 
 /* Set '-m MB' based on maxmem, because the lower 'memory' limit
  * is set post-startup using the balloon driver. If balloon driver
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index e62828e..e398d2c 100644
--- a/src/