Re: [libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration

2012-09-03 Thread Martin Kletzander 
On 08/31/2012 07:49 PM, Eric Blake wrote:
 On 08/31/2012 07:59 AM, Martin Kletzander wrote:
 This patch adds support for running qemu guests with the required
 parameters to forcefully enable or disable BIOS advertising of S3 and
 S4 states.  The support for this is added to capabilities and there is
 also a qemu command parameter parsing implemented.
 ---
  src/qemu/qemu_capabilities.c |  7 +
  src/qemu/qemu_capabilities.h |  2 ++
  src/qemu/qemu_command.c  | 62 
 
  src/qemu/qemu_driver.c   | 17 
  4 files changed, 88 insertions(+)

 +++ b/src/qemu/qemu_command.c
 @@ -4782,6 +4782,32 @@ qemuBuildCommandLine(virConnectPtr conn,
  virCommandAddArg(cmd, -no-acpi);
  }

 +if (def-pm.s3) {
 +if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   %s, _(setting ACPI S3 not supported));
 +goto error;
 +}
 +virCommandAddArgList(cmd,
 + -global,
 + (def-pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ?
 +  PIIX4_PM.disable_s3=0 : 
 PIIX4_PM.disable_s3=1),
 + NULL);
 
 Fine as is, but I probably would have written:
 
 virCommandAddArg(cmd, -global);
 virCommandAddArgFormat(cmd, PIIX4_PM.disable_s3=%d,
   def-pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED);
 
 for less typing.
 
 +++ b/src/qemu/qemu_driver.c
 @@ -13722,6 +13722,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
  goto cleanup;
  }

 +if (vm-def-pm.s3 || vm-def-pm.s4) {
 +if (!vm-def-pm.s3 == VIR_DOMAIN_PM_STATE_DISABLED 
 
 Logic bug.  (!vm-def-pm.s3) means that you have flattened an enum into
 0 or 1, before comparing it back to an enum value.  I think you meant to
 drop the ! entirely.
 
 ACK with that fix.
 

Yes, I've missed it after one fixing and haven't checked if it works
after that. Fixed, checked and pushed now. Thanks for the review.

Martin

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

[libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration

2012-08-31 Thread Martin Kletzander 
This patch adds support for running qemu guests with the required
parameters to forcefully enable or disable BIOS advertising of S3 and
S4 states.  The support for this is added to capabilities and there is
also a qemu command parameter parsing implemented.
---
 src/qemu/qemu_capabilities.c |  7 +
 src/qemu/qemu_capabilities.h |  2 ++
 src/qemu/qemu_command.c  | 62 
 src/qemu/qemu_driver.c   | 17 
 4 files changed, 88 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5472267..b8160b6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -172,6 +172,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   bridge, /* 100 */
   lsi,
   virtio-scsi-pci,
+  disable-s3,
+  disable-s4,

 );

@@ -1403,6 +1405,7 @@ qemuCapsExtractDeviceStr(const char *qemu,
  -device, virtio-blk-pci,?,
  -device, virtio-net-pci,?,
  -device, scsi-disk,?,
+ -device, PIIX4_PM,?,
  NULL);
 /* qemu -help goes to stdout, but qemu -device ? goes to stderr.  */
 virCommandSetErrorBuffer(cmd, output);
@@ -1499,6 +1502,10 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr 
flags)
 qemuCapsSet(flags, QEMU_CAPS_SCSI_CD);
 if (strstr(str, ide-cd))
 qemuCapsSet(flags, QEMU_CAPS_IDE_CD);
+if (strstr(str, PIIX4_PM.disable_s3=))
+qemuCapsSet(flags, QEMU_CAPS_DISABLE_S3);
+if (strstr(str, PIIX4_PM.disable_s4=))
+qemuCapsSet(flags, QEMU_CAPS_DISABLE_S4);

 return 0;
 }
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d606890..e49424a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -138,6 +138,8 @@ enum qemuCapsFlags {
 QEMU_CAPS_NETDEV_BRIDGE  = 100, /* bridge helper support */
 QEMU_CAPS_SCSI_LSI   = 101, /* -device lsi */
 QEMU_CAPS_VIRTIO_SCSI_PCI= 102, /* -device virtio-scsi-pci */
+QEMU_CAPS_DISABLE_S3 = 103, /* S3 BIOS Advertisement on/off */
+QEMU_CAPS_DISABLE_S4 = 104, /* S4 BIOS Advertisement on/off */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 25f2451..d4217fa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4782,6 +4782,32 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, -no-acpi);
 }

+if (def-pm.s3) {
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(setting ACPI S3 not supported));
+goto error;
+}
+virCommandAddArgList(cmd,
+ -global,
+ (def-pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ?
+  PIIX4_PM.disable_s3=0 : 
PIIX4_PM.disable_s3=1),
+ NULL);
+}
+
+if (def-pm.s4) {
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S4)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(setting ACPI S4 not supported));
+goto error;
+}
+virCommandAddArgList(cmd,
+ -global,
+ (def-pm.s4 == VIR_DOMAIN_PM_STATE_ENABLED ?
+  PIIX4_PM.disable_s4=0 : 
PIIX4_PM.disable_s4=1),
+ NULL);
+}
+
 if (!def-os.bootloader) {
 /*
  * We prefer using explicit bootindex=N parameters for predictable
@@ -8368,6 +8394,42 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,

 *monConfig = chr;
 }
+} else if (STREQ(arg, -global) 
+   STRPREFIX(progargv[i + 1], PIIX4_PM.disable_s3=)) {
+/* We want to parse only the known -global parameters,
+ * so the ones that we don't know are still added to the
+ * namespace */
+WANT_VALUE();
+
+val += strlen(PIIX4_PM.disable_s3=);
+if (STREQ(val, 0))
+def-pm.s3 = VIR_DOMAIN_PM_STATE_ENABLED;
+else if (STREQ(val, 1))
+def-pm.s3 = VIR_DOMAIN_PM_STATE_DISABLED;
+else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(invalid value for disable_s3 parameter: 
+ '%s'), val);
+goto error;
+}
+
+} else if (STREQ(arg, -global) 
+   STRPREFIX(progargv[i + 1], PIIX4_PM.disable_s4=)) {
+
+WANT_VALUE();
+
+val += strlen(PIIX4_PM.disable_s4=);
+if (STREQ(val, 0))
+def-pm.s4 = VIR_DOMAIN_PM_STATE_ENABLED;
+

Re: [libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration

2012-08-31 Thread Eric Blake 
On 08/31/2012 07:59 AM, Martin Kletzander wrote:
 This patch adds support for running qemu guests with the required
 parameters to forcefully enable or disable BIOS advertising of S3 and
 S4 states.  The support for this is added to capabilities and there is
 also a qemu command parameter parsing implemented.
 ---
  src/qemu/qemu_capabilities.c |  7 +
  src/qemu/qemu_capabilities.h |  2 ++
  src/qemu/qemu_command.c  | 62 
 
  src/qemu/qemu_driver.c   | 17 
  4 files changed, 88 insertions(+)
 
 +++ b/src/qemu/qemu_command.c
 @@ -4782,6 +4782,32 @@ qemuBuildCommandLine(virConnectPtr conn,
  virCommandAddArg(cmd, -no-acpi);
  }
 
 +if (def-pm.s3) {
 +if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +   %s, _(setting ACPI S3 not supported));
 +goto error;
 +}
 +virCommandAddArgList(cmd,
 + -global,
 + (def-pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ?
 +  PIIX4_PM.disable_s3=0 : 
 PIIX4_PM.disable_s3=1),
 + NULL);

Fine as is, but I probably would have written:

virCommandAddArg(cmd, -global);
virCommandAddArgFormat(cmd, PIIX4_PM.disable_s3=%d,
  def-pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED);

for less typing.

 +++ b/src/qemu/qemu_driver.c
 @@ -13722,6 +13722,23 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
  goto cleanup;
  }
 
 +if (vm-def-pm.s3 || vm-def-pm.s4) {
 +if (!vm-def-pm.s3 == VIR_DOMAIN_PM_STATE_DISABLED 

Logic bug.  (!vm-def-pm.s3) means that you have flattened an enum into
0 or 1, before comparing it back to an enum value.  I think you meant to
drop the ! entirely.

ACK with that fix.

-- 
Eric Blake   ebl...@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

[libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration

2012-08-23 Thread Martin Kletzander 
This patch adds support for running qemu guests with the required
parameters to forcefully enable or disable BIOS advertising of S3 and
S4 states.  The support for this is added to capabilities and there is
also a qemu command parameter parsing implemented.
---
 src/qemu/qemu_capabilities.c |  7 +
 src/qemu/qemu_capabilities.h |  2 ++
 src/qemu/qemu_command.c  | 62 
 src/qemu/qemu_driver.c   | 17 
 4 files changed, 88 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5472267..b8160b6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -172,6 +172,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   bridge, /* 100 */
   lsi,
   virtio-scsi-pci,
+  disable-s3,
+  disable-s4,

 );

@@ -1403,6 +1405,7 @@ qemuCapsExtractDeviceStr(const char *qemu,
  -device, virtio-blk-pci,?,
  -device, virtio-net-pci,?,
  -device, scsi-disk,?,
+ -device, PIIX4_PM,?,
  NULL);
 /* qemu -help goes to stdout, but qemu -device ? goes to stderr.  */
 virCommandSetErrorBuffer(cmd, output);
@@ -1499,6 +1502,10 @@ qemuCapsParseDeviceStr(const char *str, virBitmapPtr 
flags)
 qemuCapsSet(flags, QEMU_CAPS_SCSI_CD);
 if (strstr(str, ide-cd))
 qemuCapsSet(flags, QEMU_CAPS_IDE_CD);
+if (strstr(str, PIIX4_PM.disable_s3=))
+qemuCapsSet(flags, QEMU_CAPS_DISABLE_S3);
+if (strstr(str, PIIX4_PM.disable_s4=))
+qemuCapsSet(flags, QEMU_CAPS_DISABLE_S4);

 return 0;
 }
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d606890..e49424a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -138,6 +138,8 @@ enum qemuCapsFlags {
 QEMU_CAPS_NETDEV_BRIDGE  = 100, /* bridge helper support */
 QEMU_CAPS_SCSI_LSI   = 101, /* -device lsi */
 QEMU_CAPS_VIRTIO_SCSI_PCI= 102, /* -device virtio-scsi-pci */
+QEMU_CAPS_DISABLE_S3 = 103, /* S3 BIOS Advertisement on/off */
+QEMU_CAPS_DISABLE_S4 = 104, /* S4 BIOS Advertisement on/off */

 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ca62f0c..6c43696 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4781,6 +4781,32 @@ qemuBuildCommandLine(virConnectPtr conn,
 virCommandAddArg(cmd, -no-acpi);
 }

+if (def-pm.s3) {
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S3)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(setting ACPI S3 not supported));
+goto error;
+}
+virCommandAddArgList(cmd,
+ -global,
+ (def-pm.s3 == VIR_DOMAIN_PM_STATE_ENABLED ?
+  PIIX4_PM.disable_s3=0 : 
PIIX4_PM.disable_s3=1),
+ NULL);
+}
+
+if (def-pm.s4) {
+if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DISABLE_S4)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(setting ACPI S4 not supported));
+goto error;
+}
+virCommandAddArgList(cmd,
+ -global,
+ (def-pm.s4 == VIR_DOMAIN_PM_STATE_ENABLED ?
+  PIIX4_PM.disable_s4=0 : 
PIIX4_PM.disable_s4=1),
+ NULL);
+}
+
 if (!def-os.bootloader) {
 /*
  * We prefer using explicit bootindex=N parameters for predictable
@@ -8367,6 +8393,42 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,

 *monConfig = chr;
 }
+} else if (STREQ(arg, -global) 
+   STRPREFIX(progargv[i + 1], PIIX4_PM.disable_s3=)) {
+/* We want to parse only the known -global parameters,
+ * so the ones that we don't know are still added to the
+ * namespace */
+WANT_VALUE();
+
+val += strlen(PIIX4_PM.disable_s3=);
+if (STREQ(val, 0))
+def-pm.s3 = VIR_DOMAIN_PM_STATE_ENABLED;
+else if (STREQ(val, 1))
+def-pm.s3 = VIR_DOMAIN_PM_STATE_DISABLED;
+else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(invalid value for disable_s3 parameter: 
+ '%s'), val);
+goto error;
+}
+
+} else if (STREQ(arg, -global) 
+   STRPREFIX(progargv[i + 1], PIIX4_PM.disable_s4=)) {
+
+WANT_VALUE();
+
+val += strlen(PIIX4_PM.disable_s4=);
+if (STREQ(val, 0))
+def-pm.s4 = VIR_DOMAIN_PM_STATE_ENABLED;
+

Re: [libvirt] [PATCH v4 2/3] qemu: Add support for S3/S4 state configuration

2012-08-23 Thread Michal Privoznik 
On 23.08.2012 13:47, Martin Kletzander wrote:
 This patch adds support for running qemu guests with the required
 parameters to forcefully enable or disable BIOS advertising of S3 and
 S4 states.  The support for this is added to capabilities and there is
 also a qemu command parameter parsing implemented.
 ---
  src/qemu/qemu_capabilities.c |  7 +
  src/qemu/qemu_capabilities.h |  2 ++
  src/qemu/qemu_command.c  | 62 
 
  src/qemu/qemu_driver.c   | 17 
  4 files changed, 88 insertions(+)

ACK

Michal

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