Re: [libvirt] [PATCHv2 09/15] xml: use better types for memory values

2012-03-07 Thread Eric Blake
On 03/06/2012 09:34 AM, Peter Krempa wrote:
 On 03/06/2012 01:34 AM, Eric Blake wrote:
 Using 'unsigned long' for memory values is risky on 32-bit platforms,
 as a PAE guest can have more than 4GiB memory.  Our API is
 (unfortunately) locked at 'unsigned long' and a scale of 1024, but
 the rest of our system should consistently use 64-bit values,
 especially since the previous patch centralized overflow checking.


 +++ b/src/xenxs/xen_sxpr.c
 @@ -1101,9 +1133,15 @@ cleanup:


   static
 -int xenXMConfigSetInt(virConfPtr conf, const char *setting, long l) {
 +int xenXMConfigSetInt(virConfPtr conf, const char *setting, long long
 l) {
   virConfValuePtr value = NULL;

 +if ((long) l != l) {
 +XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
 
 I suppose the VIR_ERR_INTERNAL_ERROR is intentional.

Yes - the libvirt.c interface takes 'unsigned long', and widening that
to long long should be reversible so we should never hit this error.

 
 +_(integer overflow in trying to store %lld to %s),
 +l, setting);
 +return -1;
 +}
   if (VIR_ALLOC(value)  0) {
   virReportOOMError();
   return -1;
 
 ACK,
 
 Peter
 

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

Re: [libvirt] [PATCHv2 09/15] xml: use better types for memory values

2012-03-06 Thread Peter Krempa

On 03/06/2012 01:34 AM, Eric Blake wrote:

Using 'unsigned long' for memory values is risky on 32-bit platforms,
as a PAE guest can have more than 4GiB memory.  Our API is
(unfortunately) locked at 'unsigned long' and a scale of 1024, but
the rest of our system should consistently use 64-bit values,
especially since the previous patch centralized overflow checking.

* src/conf/domain_conf.h (_virDomainDef): Always use 64-bit values
for memory.  Change hugepage_backed to a bool.
* src/conf/domain_conf.c (virDomainDefParseXML)
(virDomainDefCheckABIStability, virDomainDefFormatInternal): Fix
clients.
* src/vmx/vmx.c (virVMXFormatConfig): Likewise.
* src/xenxs/xen_sxpr.c (xenParseSxpr, xenFormatSxpr): Likewise.
* src/xenxs/xen_xm.c (xenXMConfigGetULongLong): New function.
(xenXMConfigGetULong, xenXMConfigSetInt): Avoid truncation.
(xenParseXM, xenFormatXM): Fix clients.
* src/phyp/phyp_driver.c (phypBuildLpar): Likewise.
* src/openvz/openvz_driver.c (openvzDomainSetMemoryInternal):
Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Likewise.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise.
* src/qemu/qemu_process.c (qemuProcessStart): Likewise.
* src/qemu/qemu_monitor.h (qemuMonitorGetBalloonInfo): Likewise.
* src/qemu/qemu_monitor_text.h (qemuMonitorTextGetBalloonInfo):
Likewise.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBalloonInfo):
Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONGetBalloonInfo):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetBalloonInfo):
Likewise.
* src/qemu/qemu_driver.c (qemudDomainGetInfo)
(qemuDomainGetXMLDesc): Likewise.
* src/uml/uml_conf.c (umlBuildCommandLine): Likewise.
---
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index f8390ea..beafb95 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1101,9 +1133,15 @@ cleanup:


  static
-int xenXMConfigSetInt(virConfPtr conf, const char *setting, long l) {
+int xenXMConfigSetInt(virConfPtr conf, const char *setting, long long l) {
  virConfValuePtr value = NULL;

+if ((long) l != l) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,


I suppose the VIR_ERR_INTERNAL_ERROR is intentional.


+_(integer overflow in trying to store %lld to %s),
+l, setting);
+return -1;
+}
  if (VIR_ALLOC(value)  0) {
  virReportOOMError();
  return -1;


ACK,

Peter

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


[libvirt] [PATCHv2 09/15] xml: use better types for memory values

2012-03-05 Thread Eric Blake
Using 'unsigned long' for memory values is risky on 32-bit platforms,
as a PAE guest can have more than 4GiB memory.  Our API is
(unfortunately) locked at 'unsigned long' and a scale of 1024, but
the rest of our system should consistently use 64-bit values,
especially since the previous patch centralized overflow checking.

* src/conf/domain_conf.h (_virDomainDef): Always use 64-bit values
for memory.  Change hugepage_backed to a bool.
* src/conf/domain_conf.c (virDomainDefParseXML)
(virDomainDefCheckABIStability, virDomainDefFormatInternal): Fix
clients.
* src/vmx/vmx.c (virVMXFormatConfig): Likewise.
* src/xenxs/xen_sxpr.c (xenParseSxpr, xenFormatSxpr): Likewise.
* src/xenxs/xen_xm.c (xenXMConfigGetULongLong): New function.
(xenXMConfigGetULong, xenXMConfigSetInt): Avoid truncation.
(xenParseXM, xenFormatXM): Fix clients.
* src/phyp/phyp_driver.c (phypBuildLpar): Likewise.
* src/openvz/openvz_driver.c (openvzDomainSetMemoryInternal):
Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainDefineXML): Likewise.
* src/qemu/qemu_command.c (qemuBuildCommandLine): Likewise.
* src/qemu/qemu_process.c (qemuProcessStart): Likewise.
* src/qemu/qemu_monitor.h (qemuMonitorGetBalloonInfo): Likewise.
* src/qemu/qemu_monitor_text.h (qemuMonitorTextGetBalloonInfo):
Likewise.
* src/qemu/qemu_monitor_text.c (qemuMonitorTextGetBalloonInfo):
Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONGetBalloonInfo):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONGetBalloonInfo):
Likewise.
* src/qemu/qemu_driver.c (qemudDomainGetInfo)
(qemuDomainGetXMLDesc): Likewise.
* src/uml/uml_conf.c (umlBuildCommandLine): Likewise.
---

v2: new

 src/conf/domain_audit.c  |2 +-
 src/conf/domain_conf.c   |   46 
 src/conf/domain_conf.h   |   14 +-
 src/openvz/openvz_driver.c   |8 +++---
 src/phyp/phyp_driver.c   |   10 +++---
 src/qemu/qemu_command.c  |2 +-
 src/qemu/qemu_driver.c   |4 +-
 src/qemu/qemu_monitor.c  |2 +-
 src/qemu/qemu_monitor.h  |2 +-
 src/qemu/qemu_monitor_json.c |2 +-
 src/qemu/qemu_monitor_json.h |2 +-
 src/qemu/qemu_monitor_text.c |2 +-
 src/qemu/qemu_monitor_text.h |2 +-
 src/qemu/qemu_process.c  |6 
 src/uml/uml_conf.c   |2 +-
 src/vbox/vbox_tmpl.c |4 +-
 src/vmx/vmx.c|   10 +++---
 src/xenxs/xen_sxpr.c |8 ++---
 src/xenxs/xen_xm.c   |   58 +++--
 19 files changed, 115 insertions(+), 71 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 934e546..b885906 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -1,7 +1,7 @@
 /*
  * domain_audit.c: Domain audit management
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 41d0eca..fa44d3e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7292,27 +7292,27 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 goto error;

 /* Extract domain memory */
-if (virXPathULong(string(./memory[1]), ctxt,
-  def-mem.max_balloon)  0) {
+if (virXPathULongLong(string(./memory[1]), ctxt,
+  def-mem.max_balloon)  0) {
 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
  %s, _(missing memory element));
 goto error;
 }

-if (virXPathULong(string(./currentMemory[1]), ctxt,
-  def-mem.cur_balloon)  0)
+if (virXPathULongLong(string(./currentMemory[1]), ctxt,
+  def-mem.cur_balloon)  0)
 def-mem.cur_balloon = def-mem.max_balloon;

 if (def-mem.cur_balloon  def-mem.max_balloon) {
 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
- _(current memory '%luk' exceeds maximum '%luk'),
+ _(current memory '%lluk' exceeds maximum 
'%lluk'),
  def-mem.cur_balloon, def-mem.max_balloon);
 goto error;
 }

 node = virXPathNode(./memoryBacking/hugepages, ctxt);
 if (node)
-def-mem.hugepage_backed = 1;
+def-mem.hugepage_backed = true;

 /* Extract blkio cgroup tunables */
 if (virXPathUInt(string(./blkiotune/weight), ctxt,
@@ -7346,20 +7346,20 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 VIR_FREE(nodes);

 /* Extract other memory tunables */
-if (virXPathULong(string(./memtune/hard_limit), ctxt,
-  def-mem.hard_limit)  0)
+if (virXPathULongLong(string(./memtune/hard_limit), ctxt,
+  def-mem.hard_limit)  0)
 def-mem.hard_limit = 0;

-if (virXPathULong(string(./memtune/soft_limit[1]), ctxt,
-