date:20101203

[libvirt] [PATCH] Change return value of VIR_DRV_SUPPORTS_FEATURE to bool

2010-12-03 Thread Jiri Denemark

virDrvSupportsFeature API is allowed to return -1 on error while all but
one uses of VIR_DRV_SUPPORTS_FEATURE only check for (non)zero return
value. Let's make this macro return zero on error, which is what
everyone expects anyway.
---
 src/driver.h  |8 
 src/libvirt.c |5 -
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/driver.h b/src/driver.h
index b770e5e..e797a75 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -52,12 +52,12 @@ typedef enum {
  * Note that you must check for errors.
  *
  * Returns:
- *   = 1  Feature is supported.
+ *   != 0  Feature is supported.
  *   0 Feature is not supported.
- *   -1Error.
  */
-# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature)  \
-((drv)-supports_feature ? (drv)-supports_feature((conn),(feature)) : 0)
+# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
+((drv)-supports_feature ?  \
+(drv)-supports_feature((conn), (feature))  0 : 0)
 
 typedef virDrvOpenStatus
 (*virDrvOpen)  (virConnectPtr conn,
diff --git a/src/libvirt.c b/src/libvirt.c
index b4951c2..4188b45 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1605,7 +1605,10 @@ virDrvSupportsFeature (virConnectPtr conn, int feature)
 return (-1);
 }
 
-ret = VIR_DRV_SUPPORTS_FEATURE (conn-driver, conn, feature);
+if (!conn-driver-supports_feature)
+ret = 0;
+else
+ret = conn-driver-supports_feature(conn, feature);
 
 if (ret  0)
 virDispatchError(conn);
-- 
1.7.3.2

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

[libvirt] shadows a global declaration warnings in virsh

2010-12-03 Thread Justin Clift

Hi all,

Started seeing these warnings when compiling virsh:

  virsh.c: In function 'cmdRunConsole':
  virsh.c:735: warning: declaration of 'devname' shadows a global declaration 
[-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
  virsh.c: In function 'cmdConsole':
  virsh.c:765: warning: declaration of 'devname' shadows a global declaration 
[-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
  virsh.c: In function 'vshReadlineInit':
  virsh.c:11575: warning: assignment discards qualifiers from pointer target 
type

Haven't looked into it yet as they're only warnings.  Anyone know if they're 
important enough
to look into?

Regards and best wishes,

Justin Clift


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

Re: [libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Justin Clift

On 03/12/2010, at 6:34 PM, Osier Yang wrote:
 static const vshCmdOptDef opts_help[] = {
 -{command, VSH_OT_DATA, 0, N_(Prints global help or command specific 
 help.)},
 -{group, VSH_OT_DATA, 0, N_(Prints global help or help for a group of 
 related commands.)},
 +{command-or-group, VSH_OT_DATA, 0, N_(Prints global help, command 
 specific help, or help for a group of related commands)},
 {NULL, 0, 0, NULL}
 };
 
 @@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
 const vshCmdGrp *g;
 const char *name;
 
 -name = vshCommandOptString(cmd, command, NULL);
 -
 -if (!name)
 -name = vshCommandOptString(cmd, group, NULL);
 +name = vshCommandOptString(cmd, command-or-group, NULL);

NACK.  This breaks backwards compatibility for anyone with scripts already 
using --command:

  virsh # help --command domname
  error: command 'help' doesn't support option --command

With 0.8.6:

  virsh # help --command domname
NAME
  domname - convert a domain id or UUID to domain name

SYNOPSIS
  domname domain

OPTIONS
  [--domain] string  domain id or uuid

Thinking it would be better to show/have --command in the allowable options, 
but never care if it is or isn't specified.  Just make use of the option or 
group name the user gives.

?


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

Re: [libvirt] shadows a global declaration warnings in virsh

2010-12-03 Thread Osier Yang


于 2010年12月03日 16:52, Justin Clift 写道:

Hi all,

Started seeing these warnings when compiling virsh:

   virsh.c: In function 'cmdRunConsole':
   virsh.c:735: warning: declaration of 'devname' shadows a global declaration 
[-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
   virsh.c: In function 'cmdConsole':
   virsh.c:765: warning: declaration of 'devname' shadows a global declaration 
[-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
   virsh.c: In function 'vshReadlineInit':
   virsh.c:11575: warning: assignment discards qualifiers from pointer target 
type

Haven't looked into it yet as they're only warnings.  Anyone know if they're 
important enough
to look into?

it works fine for me, even I set ./confugure 
--enable-compile-warnings=maximum.


- Osier

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

Re: [libvirt] shadows a global declaration warnings in virsh

2010-12-03 Thread Justin Clift

On 03/12/2010, at 8:40 PM, Osier Yang wrote:
 于 2010年12月03日 16:52, Justin Clift 写道:
 Hi all,
 
 Started seeing these warnings when compiling virsh:
 
   virsh.c: In function 'cmdRunConsole':
   virsh.c:735: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
   virsh.c: In function 'cmdConsole':
   virsh.c:765: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]
   virsh.c: In function 'vshReadlineInit':
   virsh.c:11575: warning: assignment discards qualifiers from pointer target 
 type
 
 Haven't looked into it yet as they're only warnings.  Anyone know if they're 
 important enough
 to look into?
 
 it works fine for me, even I set ./confugure 
 --enable-compile-warnings=maximum.

Sorry, should have mentioned that they're only showing up in OSX, but not in 
Fedora 13. :)

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

[libvirt] [Patch v2] Create file in virFileWriteStr() if it doesn't exist

2010-12-03 Thread Jean-Baptiste Rouault

This patch adds a mode_t parameter to virFileWriteStr().
If mode is different from 0, virFileWriteStr() will try
to create the file if it doesn't exist.
---
 src/network/bridge_driver.c  |8 
 src/node_device/node_device_driver.c |2 +-
 src/util/cgroup.c|2 +-
 src/util/pci.c   |   16 
 src/util/util.c  |   11 ---
 src/util/util.h  |2 +-
 6 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 54890f9..62639e4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1024,7 +1024,7 @@ networkReloadIptablesRules(struct network_driver *driver)
 static int
 networkEnableIpForwarding(void)
 {
-return virFileWriteStr(/proc/sys/net/ipv4/ip_forward, 1\n);
+return virFileWriteStr(/proc/sys/net/ipv4/ip_forward, 1\n, 0);
 }
 
 #define SYSCTL_PATH /proc/sys
@@ -1045,7 +1045,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
 goto cleanup;
 }
 
-if (virFileWriteStr(field, 1)  0) {
+if (virFileWriteStr(field, 1, 0)  0) {
 virReportSystemError(errno,
  _(cannot enable %s), field);
 goto cleanup;
@@ -1057,7 +1057,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
 goto cleanup;
 }
 
-if (virFileWriteStr(field, 0)  0) {
+if (virFileWriteStr(field, 0, 0)  0) {
 virReportSystemError(errno,
  _(cannot disable %s), field);
 goto cleanup;
@@ -1069,7 +1069,7 @@ static int networkDisableIPV6(virNetworkObjPtr network)
 goto cleanup;
 }
 
-if (virFileWriteStr(field, 1)  0) {
+if (virFileWriteStr(field, 1, 0)  0) {
 virReportSystemError(errno,
  _(cannot enable %s), field);
 goto cleanup;
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index 448cfd3..a6c6042 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -454,7 +454,7 @@ nodeDeviceVportCreateDelete(const int parent_host,
 goto cleanup;
 }
 
-if (virFileWriteStr(operation_path, vport_name) == -1) {
+if (virFileWriteStr(operation_path, vport_name, 0) == -1) {
 virReportSystemError(errno,
  _(Write of '%s' to '%s' during 
vport create/delete failed),
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index 2758a8f..3ba6325 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -288,7 +288,7 @@ static int virCgroupSetValueStr(virCgroupPtr group,
 return rc;
 
 VIR_DEBUG(Set value '%s' to '%s', keypath, value);
-rc = virFileWriteStr(keypath, value);
+rc = virFileWriteStr(keypath, value, 0);
 if (rc  0) {
 DEBUG(Failed to write value '%s': %m, value);
 rc = -errno;
diff --git a/src/util/pci.c b/src/util/pci.c
index d38cefa..095ad3f 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -849,7 +849,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
  * bound by the stub.
  */
 pciDriverFile(path, sizeof(path), driver, new_id);
-if (virFileWriteStr(path, dev-id)  0) {
+if (virFileWriteStr(path, dev-id, 0)  0) {
 virReportSystemError(errno,
  _(Failed to add PCI device ID '%s' to %s),
  dev-id, driver);
@@ -862,7 +862,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
  * your root filesystem.
  */
 pciDeviceFile(path, sizeof(path), dev-name, driver/unbind);
-if (virFileExists(path)  virFileWriteStr(path, dev-name)  0) {
+if (virFileExists(path)  virFileWriteStr(path, dev-name, 0)  0) {
 virReportSystemError(errno,
  _(Failed to unbind PCI device '%s'), dev-name);
 return -1;
@@ -875,7 +875,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
 if (!virFileLinkPointsTo(path, drvdir)) {
 /* Xen's pciback.ko wants you to use new_slot first */
 pciDriverFile(path, sizeof(path), driver, new_slot);
-if (virFileExists(path)  virFileWriteStr(path, dev-name)  0) {
+if (virFileExists(path)  virFileWriteStr(path, dev-name, 0)  0) {
 virReportSystemError(errno,
  _(Failed to add slot for PCI device '%s' to 
%s),
  dev-name, driver);
@@ -883,7 +883,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver)
 }
 
 pciDriverFile(path, sizeof(path), driver, bind);
-if (virFileWriteStr(path, dev-name)  0) {
+if (virFileWriteStr(path, dev-name, 0)  0) {
 virReportSystemError(errno,
  _(Failed to bind PCI device '%s' to %s),
  dev-name, driver);
@@ -895,7 +895,7 @@

Re: [libvirt] [PATCH 0/2] smbios fixes

2010-12-03 Thread Gleb Natapov

On Thu, Dec 02, 2010 at 08:12:23PM +, Daniel P. Berrange wrote:
 On Thu, Dec 02, 2010 at 10:04:59PM +0200, Gleb Natapov wrote:
  On Thu, Dec 02, 2010 at 07:56:03PM +, Daniel P. Berrange wrote:
   On Thu, Dec 02, 2010 at 09:44:47PM +0200, Gleb Natapov wrote:
On Thu, Dec 02, 2010 at 10:36:59AM -0700, Eric Blake wrote:
 On 12/02/2010 05:47 AM, Daniel P. Berrange wrote:
  On Wed, Dec 01, 2010 at 05:12:08PM -0700, Eric Blake wrote:
  In testing smbios mode='host'/, I noticed a couple of problems.
  First, qemu rejected the command line we gave it (invalid UUID);
  ifixingthat showed that all other arguments were being given 
  literal
   which then made the guest smbios differ from the host.  Second,
  the qemu option -smbios type=1,family=string wasn't supported, 
  which
  lead to a spurious difference between host and guest.
 
  Meanwhile, qemu has a bug where '-smbios type=1,uuid=uuid' must 
  parse
  as a valid uuid, but is otherwise ignored.  The current qemu.git 
  code
  base _only_ sets smbios uuid from the '-uuid uuid' argument.  I've
  filed a bug against the qemu folks asking whether this is intended 
  (in
  which case we have to modify libvirt to change the -uuid argument 
  it
  outputs when smbios is specified), or an oversight (hopefully the
  latter, since it's still nice to correlate
  /var/log/libvirt/qemu/log with the XML uuid even when that 
  differs
  from the smbios uuid presented to the guest.)
  
  Hmm, I thought the UUID we were passing in was a host UUID, but
  on closer inspection the type=1 table is definitely intended to
  carry the guest UUID. As such it doesn't make sense to populate
  that with anything other than the 'uuid' from the guest XML.
  A host UUID should live elsewhere in the SMBIOS tables, likely
  in the type2 or type3 blocks
 
 What does that mean to our XML?  Should we reject XML files where both
 domain/uuid and domain/sysinfo/system/entry/uuid exist, but differ?
 Both elements are optional, so it's feasible to see a guest uuid in
 either location.  Meanwhile, I'm waiting on resolution to
 https://bugzilla.redhat.com/show_bug.cgi?id=659122 to see if qemu will
 be patched to let us stick the host's uuid in block 2 (base board) or
 block 3 (chassis), in which case, we'll need to expand our XML to
 support that notion.
 
As I commented on the BZ use OEM strings type 11 smbios table to pass
anything you want into a guest.

 I've also discovered that with current qemu, if both '-uuid uuid' and
 '-smbios type=1,uuid=uuid' are specified, then -uuid takes precedence;
 but either one of the two in isolation serves to set smbios block 1 
 with
 the guest's uuid.
 
I am surprised that libvirt still uses -uuid.
   
   SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt.
   On non-x86, or QEMU used for Xen paravirt, the -uuid arg value
   would be used in other ways unrelated to SMBIOS. Thus replacing
   -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong.
   
  What non-x86 platforms libvirt supports? With Xen use -uuid or whatever
  they support. With KVM use only smbios type=1,uuid=$UUID. There is not
  valid reason that I see to use both. But if you use both of them for some
  strange reason please make sure you provide the same value for both.
 
 libvirt aims to support any and all QEMU target architectures
 not merely x86. There's no benefit to using -smbios type=1,uuid=$UUID
 with KVM, over -uuid $UUID.
So why do you use them both? Even more interesting question why do you
use them both with different $UUID? If you do, do not complain that there
should be some kind of order between them.

  Changing it only for KVM would needlessly
 complicate the code.
 
Doing things correctly is more important that simple code. And this is
not related to KVM at all. You cannot expect qemu-sparc to work
with qemu-x86 command line, so if you aim to support any and all qemu
targets you should know how to build correct parameter list for any and
all of them.

--
Gleb.

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

Re: [libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Osier Yang



-name = vshCommandOptString(cmd, command, NULL);
-
-if (!name)
-name = vshCommandOptString(cmd, group, NULL);
+name = vshCommandOptString(cmd, command-or-group, NULL);


NACK.  This breaks backwards compatibility for anyone with scripts already using 
--command:



Don't think anyone really used --command, our test programs
even doesn't use it.

So then it's reasonable to be consistent, as virsh supports
both command and command group now, and actually I save
you pushed the doc patch.

Regards

- Osier

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

[libvirt] [PATCH] qemu: Fix a possible deadlock in p2p migration

2010-12-03 Thread Jiri Denemark

Two more calls to remote libvirtd have to be surrounded by
qemuDomainObjEnterRemoteWithDriver() and
qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between
two communicating libvirt daemons.

See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details.
---
 src/qemu/qemu_driver.c |   18 --
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 54e9dcb..bc506c2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11622,24 +11622,38 @@ static int doPeer2PeerMigrate(virDomainPtr dom,
 int ret = -1;
 virConnectPtr dconn = NULL;
 char *dom_xml;
+bool p2p;
 
 /* the order of operations is important here; we make sure the
  * destination side is completely setup before we touch the source
  */
 
+qemuDomainObjEnterRemoteWithDriver(driver, vm);
 dconn = virConnectOpen(uri);
+qemuDomainObjExitRemoteWithDriver(driver, vm);
 if (dconn == NULL) {
 qemuReportError(VIR_ERR_OPERATION_FAILED,
 _(Failed to connect to remote libvirt URI %s), uri);
 return -1;
 }
-if (!VIR_DRV_SUPPORTS_FEATURE(dconn-driver, dconn,
-  VIR_DRV_FEATURE_MIGRATION_P2P)) {
+
+qemuDomainObjEnterRemoteWithDriver(driver, vm);
+p2p = VIR_DRV_SUPPORTS_FEATURE(dconn-driver, dconn,
+   VIR_DRV_FEATURE_MIGRATION_P2P);
+qemuDomainObjExitRemoteWithDriver(driver, vm);
+if (!p2p) {
 qemuReportError(VIR_ERR_OPERATION_FAILED, %s,
 _(Destination libvirt does not support peer-to-peer 
migration protocol));
 goto cleanup;
 }
 
+/* domain may have been stopped while we were talking to remote daemon */
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR, %s,
+_(guest unexpectedly quit));
+goto cleanup;
+}
+
 dom_xml = qemudVMDumpXML(driver, vm,
  VIR_DOMAIN_XML_SECURE |
  VIR_DOMAIN_XML_UPDATE_CPU);
-- 
1.7.3.2

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

Re: [libvirt] [Patch v2] Create file in virFileWriteStr() if it doesn't exist

2010-12-03 Thread Jean-Baptiste Rouault

Forget that patch, I didn't receive it by email yet and I saw
Eric patch on the list archive after sending it...

Regards,

-- 
Jean-Baptiste ROUAULT
Ingénieur RD - Diateam : Architectes de l'information
Phone : +33 (0)9 53 16 02 70 Fax : +33 (0)2 98 050 051

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

Re: [libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Justin Clift

On 03/12/2010, at 8:33 PM, Osier Yang wrote:
 Don't think anyone really used --command, our test programs
 even doesn't use it.

Yeah.  The problem is that we don't know, even if it's unlikely.

I'm thinking removing the option, when it's not important that we
do, is probably the safer idea.

 So then it's reasonable to be consistent, as virsh supports
 both command and command group now, and actually I save
 you pushed the doc patch.

Sure.  And it's easy to update again as well. ;)

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

Re: [libvirt] shadows a global declaration warnings in virsh

2010-12-03 Thread Matthias Bolte

2010/12/3 Justin Clift jcl...@redhat.com:
 On 03/12/2010, at 8:40 PM, Osier Yang wrote:
 于 2010年12月03日 16:52, Justin Clift 写道:
 Hi all,

 Started seeing these warnings when compiling virsh:

   virsh.c: In function 'cmdRunConsole':
   virsh.c:735: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here 
 [-Wshadow]
   virsh.c: In function 'cmdConsole':
   virsh.c:765: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here 
 [-Wshadow]
   virsh.c: In function 'vshReadlineInit':
   virsh.c:11575: warning: assignment discards qualifiers from pointer 
 target type

 Haven't looked into it yet as they're only warnings.  Anyone know if 
 they're important enough
 to look into?

 it works fine for me, even I set ./confugure 
 --enable-compile-warnings=maximum.

 Sorry, should have mentioned that they're only showing up in OSX, but not in 
 Fedora 13. :)


IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it yet.

Matthias

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

Re: [libvirt] shadows a global declaration warnings in virsh

2010-12-03 Thread Jiri Denemark

virsh.c: In function 'cmdRunConsole':
virsh.c:735: warning: declaration of 'devname' shadows a global 
  declaration [-Wshadow]
/usr/include/stdlib.h:290: warning: shadowed declaration is here 
  [-Wshadow]
virsh.c: In function 'cmdConsole':
virsh.c:765: warning: declaration of 'devname' shadows a global 
  declaration [-Wshadow]
/usr/include/stdlib.h:290: warning: shadowed declaration is here 
  [-Wshadow]
virsh.c: In function 'vshReadlineInit':
virsh.c:11575: warning: assignment discards qualifiers from pointer 
  target type
  
  Haven't looked into it yet as they're only warnings.  Anyone know if 
  they're important enough
  to look into?

Well, all warnings are important since we should be able to build with
--enable-compile-warnings=error

 Sorry, should have mentioned that they're only showing up in OSX, but not in 
 Fedora 13. :)

Yeah, I was thinking it was the case. Apparently, OSX has devname declared in
/usr/include/stdlib.h:290. I wonder if it's only OSX or other BSD systems are
affected as well.

Jirka

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

Re: [libvirt] shadows a global declaration warnings in virsh

2010-12-03 Thread Justin Clift

On 03/12/2010, at 9:04 PM, Jiri Denemark wrote:
  virsh.c: In function 'cmdRunConsole':
  virsh.c:735: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here 
 [-Wshadow]
  virsh.c: In function 'cmdConsole':
  virsh.c:765: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here 
 [-Wshadow]
  virsh.c: In function 'vshReadlineInit':
  virsh.c:11575: warning: assignment discards qualifiers from pointer 
 target type
 
 Haven't looked into it yet as they're only warnings.  Anyone know if 
 they're important enough
 to look into?
 
 Well, all warnings are important since we should be able to build with
 --enable-compile-warnings=error
 
 Sorry, should have mentioned that they're only showing up in OSX, but not in 
 Fedora 13. :)
 
 Yeah, I was thinking it was the case. Apparently, OSX has devname declared in
 /usr/include/stdlib.h:290. I wonder if it's only OSX or other BSD systems are
 affected as well.

Looks like it from what Matthias just mentioned.  Looking in 
/usr/include/stdlib.h on the box here,
it shows these:

int  daemon(int, int) __DARWIN_1050(daemon) 
__OSX_AVAILABLE_BUT_DEPRECATED(__MAC_10_0,__MAC_10_5,__IPHONE_2_0,__IPHONE_2_0);
char*devname(dev_t, mode_t);
char*devname_r(dev_t, mode_t, char *buf, int len);
char*getbsize(int *, long *);
int  getloadavg(double [], int);
const char
*getprogname(void);

(included a few extra lines for context)

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

Re: [libvirt] directory storage pools reported available space

2010-12-03 Thread Ryan Harper

* Osier Yang jy...@redhat.com [2010-12-03 00:23]:
 ??? 2010???12???03??? 14:00, Ryan Harper ??:
 We recently had an issue with not being able to allocate the full
 capacity of a directory based storage pool.  The reported value via
 pool-info was larger than what was available to the image creator.
 
 Looking at the storage code, in virStorageBackendFileSystemRefresh()
 we're using statvfs, and reporting back
 
 pool-def-available = ((unsigned long long)sb.f_bfree *
(unsigned long long)sb.f_bsize);
 
 Which is the amount of blocks available, including any root reservation
 if present on the filesystem.
 
 This does't line up with df output , which at least on RHEL5 and 6
 systems reports the available space from f_bavail, which excludes
 and reserved space.
 
 Is it reasonable to have the available value line up with output from df
 and not report reserved space?
 
 It's misleading not to exclude the reserved space, probly it will be
 nicer to report both the actually avaiable spaces and the reserved
 ones.

I argue the opposite.  df doesn't show you the reserved space.  the
first thing someone does to compare the values between libvirt directory
pool and df.

I don't mind reporting both but, I've yet to see a tool to report the
reserved value rather than the non-reserved.


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

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com

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

Re: [libvirt] directory storage pools reported available space

2010-12-03 Thread Justin Clift

On 04/12/2010, at 1:29 AM, Ryan Harper wrote:
 It's misleading not to exclude the reserved space, probly it will be
 nicer to report both the actually avaiable spaces and the reserved
 ones.
 
 I argue the opposite.  df doesn't show you the reserved space.  the
 first thing someone does to compare the values between libvirt directory
 pool and df.
 
 I don't mind reporting both but, I've yet to see a tool to report the
 reserved value rather than the non-reserved.

Yeah, personally I'm in agreement with Ryan here.  The reserved amount
is mostly a throw-back concept not often used these days.  SysAdmins
generally kind of expect all of the free space to be reported/available
for use.  If we can only show one value, it should be the whole amount,
not excluding reserved space.

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

Re: [libvirt] [PATCH] Change return value of VIR_DRV_SUPPORTS_FEATURE to bool

2010-12-03 Thread Eric Blake

On 12/03/2010 01:48 AM, Jiri Denemark wrote:
 virDrvSupportsFeature API is allowed to return -1 on error while all but
 one uses of VIR_DRV_SUPPORTS_FEATURE only check for (non)zero return
 value. Let's make this macro return zero on error, which is what
 everyone expects anyway.
 ---
  src/driver.h  |8 
  src/libvirt.c |5 -
  2 files changed, 8 insertions(+), 5 deletions(-)

That is, if you care about the possibility of an error, call the
function directly; and if you use the macro, then you don't care about
errors and are happy treating an error the same as not present.

Seems reasonable.

 
 diff --git a/src/driver.h b/src/driver.h
 index b770e5e..e797a75 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -52,12 +52,12 @@ typedef enum {
   * Note that you must check for errors.

However, this comment is no longer applicable.  So you'll need a v2 that
fixes the documentation, and make it explicit that the macro ignores
errors, as well as a cross-reference to the actual function for someone
that cares about errors.

   *
   * Returns:
 - *   = 1  Feature is supported.
 + *   != 0  Feature is supported.
   *   0 Feature is not supported.
 - *   -1Error.
   */
 -# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature)  \
 -((drv)-supports_feature ? (drv)-supports_feature((conn),(feature)) : 0)
 +# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
 +((drv)-supports_feature ?  \
 +(drv)-supports_feature((conn), (feature))  0 : 0)
  

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] qemu: Fix a possible deadlock in p2p migration

2010-12-03 Thread Eric Blake

On 12/03/2010 02:53 AM, Jiri Denemark wrote:
 Two more calls to remote libvirtd have to be surrounded by
 qemuDomainObjEnterRemoteWithDriver() and
 qemuDomainObjExitRemoteWithDriver() to prevent possible deadlock between
 two communicating libvirt daemons.
 
 See commit f0c8e1cb3774d6f09e2681ca1988bf235a343007 for further details.
 ---
  src/qemu/qemu_driver.c |   18 --
  1 files changed, 16 insertions(+), 2 deletions(-)

ACK.

 -if (!VIR_DRV_SUPPORTS_FEATURE(dconn-driver, dconn,
 -  VIR_DRV_FEATURE_MIGRATION_P2P)) {
 +
 +qemuDomainObjEnterRemoteWithDriver(driver, vm);
 +p2p = VIR_DRV_SUPPORTS_FEATURE(dconn-driver, dconn,
 +   VIR_DRV_FEATURE_MIGRATION_P2P);

Ah - this explains your patch for VIR_DRV_SUPPORTS_FEATURE; and this is
indeed a case where we want error to be equated with no feature support.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 v2] Create file in virFileWriteStr() if it doesn't exist

2010-12-03 Thread Eric Blake

On 12/03/2010 02:58 AM, Jean-Baptiste Rouault wrote:
 Forget that patch, I didn't receive it by email yet and I saw
 Eric patch on the list archive after sending it...

Seeing as how your patch is practically identical to mine, I'm taking
that as an ACK, and pushing in your name.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Eric Blake

On 12/03/2010 02:17 AM, Justin Clift wrote:
 On 03/12/2010, at 6:34 PM, Osier Yang wrote:
 static const vshCmdOptDef opts_help[] = {
 -{command, VSH_OT_DATA, 0, N_(Prints global help or command specific 
 help.)},
 -{group, VSH_OT_DATA, 0, N_(Prints global help or help for a group of 
 related commands.)},
 +{command-or-group, VSH_OT_DATA, 0, N_(Prints global help, command 
 specific help, or help for a group of related commands)},
 {NULL, 0, 0, NULL}
 };

 @@ -581,10 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
 const vshCmdGrp *g;
 const char *name;

 -name = vshCommandOptString(cmd, command, NULL);
 -
 -if (!name)
 -name = vshCommandOptString(cmd, group, NULL);
 +name = vshCommandOptString(cmd, command-or-group, NULL);
 
 NACK.  This breaks backwards compatibility for anyone with scripts already 
 using --command:

Two points:
1. it is very unlikely that any scripts were already using --command,
since 'virsh help --command xyz' has always been equivalent to 'virsh
help xyz', and few people type a longer command when a short will do.
2. if we install unambiguous prefix parsing, then --command is an
unambiguous prefix of --command-or-group.

For now, since there has been no release with --group, I think the
better course of action is to just keep the name --command.  Only when
we introduce abbreviation support can we lengthen it to --command-or-group.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Eric Blake

On 12/03/2010 12:34 AM, Osier Yang wrote:
 Remove the optional option group, as cmdHelp should accepts
 only one option, and rename option command as command-or-group,
 (virsh help supports both command and command group now, and user
 nealy uses the options, so it doesn't matter much for it being longer,
 :-)
 
 * tools/virsh.c
 ---
  tools/virsh.c |8 ++--
  1 files changed, 2 insertions(+), 6 deletions(-)

Given the ensuing discussion, I'm pushing with this squashed in.

diff --git i/tools/virsh.c w/tools/virsh.c
index c2d717f..3e1bde1 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -569,7 +569,7 @@ static const vshCmdInfo info_help[] = {
 };

 static const vshCmdOptDef opts_help[] = {
-{command-or-group, VSH_OT_DATA, 0, N_(Prints global help,
command specific help, or help for a group of related commands)},
+{command, VSH_OT_DATA, 0, N_(Prints global help, command
specific help, or help for a group of related commands)},
 {NULL, 0, 0, NULL}
 };

@@ -580,7 +580,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
 const vshCmdGrp *g;
 const char *name;

-name = vshCommandOptString(cmd, command-or-group, NULL);
+name = vshCommandOptString(cmd, command, NULL);

 if (!name) {
 const vshCmdGrp *grp;

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] build: avoid shadowing devname() on BSD systems

2010-12-03 Thread Eric Blake

* tools/virsh.c (cmdRunConsole, cmdConsole): Rename problematic
symbol.
Reported by Justin Clift.
---
 IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it 
 yet.
Pushing under the build-breaker rule, then.

 tools/virsh.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 3e1bde1..441fd77 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -732,7 +732,7 @@ static const vshCmdOptDef opts_console[] = {
 };

 static int
-cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *devname)
+cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *name)
 {
 int ret = FALSE;
 virDomainInfo dominfo;
@@ -749,7 +749,7 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char 
*devname)

 vshPrintExtra(ctl, _(Connected to domain %s\n), virDomainGetName(dom));
 vshPrintExtra(ctl, %s, _(Escape character is ^]\n));
-if (vshRunConsole(dom, devname) == 0)
+if (vshRunConsole(dom, name) == 0)
 ret = TRUE;

  cleanup:
@@ -762,7 +762,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
 {
 virDomainPtr dom;
 int ret;
-const char *devname;
+const char *name;

 if (!vshConnectionUsability(ctl, ctl-conn))
 return FALSE;
@@ -770,9 +770,9 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd)
 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
 return FALSE;

-devname = vshCommandOptString(cmd, devname, NULL);
+name = vshCommandOptString(cmd, devname, NULL);

-ret = cmdRunConsole(ctl, dom, devname);
+ret = cmdRunConsole(ctl, dom, name);

 virDomainFree(dom);
 return ret;
-- 
1.7.3.2

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

Re: [libvirt] [PATCH] Change return value of VIR_DRV_SUPPORTS_FEATURE to bool

2010-12-03 Thread Jiri Denemark

  diff --git a/src/driver.h b/src/driver.h
  index b770e5e..e797a75 100644
  --- a/src/driver.h
  +++ b/src/driver.h
  @@ -52,12 +52,12 @@ typedef enum {
* Note that you must check for errors.
 
 However, this comment is no longer applicable.  So you'll need a v2 that
 fixes the documentation, and make it explicit that the macro ignores
 errors, as well as a cross-reference to the actual function for someone
 that cares about errors.

Ah, I haven't noticed that line... What about the following v2?

Jirka

From 592fd277d7a853025d92946701590485c0973205 Mon Sep 17 00:00:00 2001
Message-Id: 
592fd277d7a853025d92946701590485c0973205.1291390396.git.jdene...@redhat.com
From: Jiri Denemark jdene...@redhat.com
Date: Fri, 3 Dec 2010 09:31:48 +0100
Subject: [PATCH] Change return value of VIR_DRV_SUPPORTS_FEATURE to bool
Mail-Followup-To: libvir-list@redhat.com

virDrvSupportsFeature API is allowed to return -1 on error while all but
one uses of VIR_DRV_SUPPORTS_FEATURE only check for (non)zero return
value. Let's make this macro return zero on error, which is what
everyone expects anyway.
---
 src/driver.h  |   15 +--
 src/libvirt.c |5 -
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/driver.h b/src/driver.h
index b770e5e..75305fe 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -47,17 +47,20 @@ typedef enum {
 
 
 /* Internal feature-detection macro.  Don't call drv-supports_feature
- * directly, because it may be NULL, use this macro instead.
+ * directly if you don't have to, because it may be NULL, use this macro
+ * instead.
  *
- * Note that you must check for errors.
+ * Note that this treats a possible error returned by drv-supports_feature
+ * the same as not supported. If you care about the error, call
+ * drv-supports_feature directly.
  *
  * Returns:
- *   = 1  Feature is supported.
+ *   != 0  Feature is supported.
  *   0 Feature is not supported.
- *   -1Error.
  */
-# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature)  \
-((drv)-supports_feature ? (drv)-supports_feature((conn),(feature)) : 0)
+# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
+((drv)-supports_feature ?  \
+(drv)-supports_feature((conn), (feature))  0 : 0)
 
 typedef virDrvOpenStatus
 (*virDrvOpen)  (virConnectPtr conn,
diff --git a/src/libvirt.c b/src/libvirt.c
index b4951c2..4188b45 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1605,7 +1605,10 @@ virDrvSupportsFeature (virConnectPtr conn, int feature)
 return (-1);
 }
 
-ret = VIR_DRV_SUPPORTS_FEATURE (conn-driver, conn, feature);
+if (!conn-driver-supports_feature)
+ret = 0;
+else
+ret = conn-driver-supports_feature(conn, feature);
 
 if (ret  0)
 virDispatchError(conn);
-- 
1.7.3.2

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

Re: [libvirt] [PATCH] Change return value of VIR_DRV_SUPPORTS_FEATURE to bool

2010-12-03 Thread Eric Blake

On 12/03/2010 08:36 AM, Jiri Denemark wrote:
 diff --git a/src/driver.h b/src/driver.h
 index b770e5e..e797a75 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -52,12 +52,12 @@ typedef enum {
   * Note that you must check for errors.

 However, this comment is no longer applicable.  So you'll need a v2 that
 fixes the documentation, and make it explicit that the macro ignores
 errors, as well as a cross-reference to the actual function for someone
 that cares about errors.
 
 Ah, I haven't noticed that line... What about the following v2?

ACK.

  /* Internal feature-detection macro.  Don't call drv-supports_feature
 - * directly, because it may be NULL, use this macro instead.
 + * directly if you don't have to, because it may be NULL, use this macro
 + * instead.
   *
 - * Note that you must check for errors.
 + * Note that this treats a possible error returned by drv-supports_feature
 + * the same as not supported. If you care about the error, call
 + * drv-supports_feature directly.
   *

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] build: Fix internal docs generation in VPATH builds

2010-12-03 Thread Jiri Denemark

---
 docs/Makefile.am |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index ce0b391..43c54f6 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -117,6 +117,7 @@ todo:
 internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in
@if [ -x $(XSLTPROC) ] ; then \
  echo Generating $@; \
+ $(MKDIR_P) $(builddir)/internals; \
  name=`echo $@ | sed -e 's/.tmp//'`; \
  $(XSLTPROC) --stringparam pagename $$name --nonet --html \
$(top_srcdir)/docs/subsite.xsl $  $@ \
-- 
1.7.3.2

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

[libvirt] [PATCH] tests: Don't ignore return value of getcwd()

2010-12-03 Thread Jiri Denemark

---
 tests/commandhelper.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 2ee9153..5b2f301 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -97,7 +97,8 @@ int main(int argc, char **argv) {
 
 fprintf(log, DAEMON:%s\n, getppid() == 1 ? yes : no);
 char cwd[1024];
-getcwd(cwd, sizeof(cwd));
+if (!getcwd(cwd, sizeof(cwd)))
+return EXIT_FAILURE;
 if (strlen(cwd)  strlen(/commanddata) 
 STREQ(cwd + strlen(cwd) - strlen(/commanddata), /commanddata))
 strcpy(cwd, .../commanddata);
-- 
1.7.3.2

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

Re: [libvirt] [PATCH] build: Fix internal docs generation in VPATH builds

2010-12-03 Thread Eric Blake

On 12/03/2010 09:08 AM, Jiri Denemark wrote:
 ---
  docs/Makefile.am |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

ACK.

 
 diff --git a/docs/Makefile.am b/docs/Makefile.am
 index ce0b391..43c54f6 100644
 --- a/docs/Makefile.am
 +++ b/docs/Makefile.am
 @@ -117,6 +117,7 @@ todo:
  internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl 
 sitemap.html.in
   @if [ -x $(XSLTPROC) ] ; then \
 echo Generating $@; \
 +   $(MKDIR_P) $(builddir)/internals; \
 name=`echo $@ | sed -e 's/.tmp//'`; \
 $(XSLTPROC) --stringparam pagename $$name --nonet --html \
   $(top_srcdir)/docs/subsite.xsl $  $@ \

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] tests: Fix commandtest in VPATH build

2010-12-03 Thread Jiri Denemark

---
 tests/commandtest.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/tests/commandtest.c b/tests/commandtest.c
index ace2f33..31adeea 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -509,9 +509,12 @@ cleanup:
  * Only stdin/out/err open
  */
 static int test15(const void *unused ATTRIBUTE_UNUSED) {
+char *cwd = NULL;
 virCommandPtr cmd = virCommandNew(abs_builddir /commandhelper);
 
-virCommandSetWorkingDirectory(cmd, abs_builddir /commanddata);
+if (virAsprintf(cwd, %s/commanddata, abs_srcdir)  0)
+return -1;
+virCommandSetWorkingDirectory(cmd, cwd);
 
 if (virCommandRun(cmd, NULL)  0) {
 virErrorPtr err = virGetLastError();
@@ -519,6 +522,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) {
 return -1;
 }
 
+VIR_FREE(cwd);
 virCommandFree(cmd);
 
 return checkoutput(test15);
-- 
1.7.3.2

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

Re: [libvirt] [PATCH] tests: Don't ignore return value of getcwd()

2010-12-03 Thread Eric Blake

On 12/03/2010 09:11 AM, Jiri Denemark wrote:
 ---
  tests/commandhelper.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

ACK.

 
 diff --git a/tests/commandhelper.c b/tests/commandhelper.c
 index 2ee9153..5b2f301 100644
 --- a/tests/commandhelper.c
 +++ b/tests/commandhelper.c
 @@ -97,7 +97,8 @@ int main(int argc, char **argv) {
  
  fprintf(log, DAEMON:%s\n, getppid() == 1 ? yes : no);
  char cwd[1024];
 -getcwd(cwd, sizeof(cwd));
 +if (!getcwd(cwd, sizeof(cwd)))
 +return EXIT_FAILURE;
  if (strlen(cwd)  strlen(/commanddata) 
  STREQ(cwd + strlen(cwd) - strlen(/commanddata), /commanddata))
  strcpy(cwd, .../commanddata);

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] build: avoid shadowing devname() on BSD systems

2010-12-03 Thread Justin Clift

On 04/12/2010, at 2:30 AM, Eric Blake wrote:
 * tools/virsh.c (cmdRunConsole, cmdConsole): Rename problematic
 symbol.
 Reported by Justin Clift.
 ---
 IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it 
 yet.
 Pushing under the build-breaker rule, then.
 
 tools/virsh.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

Thanks Eric.  Captured the complete make log this time, to ensure it's fixed.

The same warning also appears in a few other places. :/

  libvirt.c: In function 'virDomainOpenConsole':
  libvirt.c:13169: warning: declaration of 'devname' shadows a global 
declaration [-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]

  remote/remote_driver.c: In function 'remoteDomainOpenConsole':
  remote/remote_driver.c:9250: warning: declaration of 'devname' shadows a 
global declaration [-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]

  console.c: In function 'vshRunConsole':
  console.c:279: warning: declaration of 'devname' shadows a global declaration 
[-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]

  libvirt.c: In function 'libvirt_virDomainOpenConsole':
  libvirt.c:1255: warning: declaration of 'devname' shadows a global 
declaration [-Wshadow]
  /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]

While looking for those, a few more warnings also showed up:

  ../daemon/event.c: In function 'virEventInterruptLocked':
  ../daemon/event.c:656: warning: cast from pointer to integer of different 
size [-Wpointer-to-int-cast]

  virsh.c: In function 'vshReadlineInit':
  virsh.c:11575: warning: assignment discards qualifiers from pointer target 
type

  event.c: In function 'virEventInterruptLocked':
  event.c:656: warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]

Again though, they're warnings and don't stop the compilation from completing.


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

Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems

2010-12-03 Thread Matthias Bolte

2010/12/3 Eric Blake ebl...@redhat.com:
 * tools/virsh.c (cmdRunConsole, cmdConsole): Rename problematic
 symbol.
 Reported by Justin Clift.
 ---
 IIRC, it had the same warning on FreeBSD, I just didn't post a patch for it 
 yet.
 Pushing under the build-breaker rule, then.


Actually on FreeBSD this shadowing problem doesn't affect virsh.c
only, but libvirt itself too. For example, the virDomainOpenConsole
function takes a devname argument.

I should probably try to compile current git on FreeBSD again.

Matthias

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

Re: [libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Justin Clift

On 04/12/2010, at 2:24 AM, Eric Blake wrote:
 On 12/03/2010 12:34 AM, Osier Yang wrote:
 Remove the optional option group, as cmdHelp should accepts
 only one option, and rename option command as command-or-group,
 (virsh help supports both command and command group now, and user
 nealy uses the options, so it doesn't matter much for it being longer,
 :-)
 
 * tools/virsh.c
 ---
 tools/virsh.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)
 
 Given the ensuing discussion, I'm pushing with this squashed in.

Thanks Eric, that works well.  I'll push a matching tweak to the man page in a
minute too (under the trivial rule).


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

Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems

2010-12-03 Thread Diego Elio Pettenò

Il giorno sab, 04/12/2010 alle 04.08 +1100, Justin Clift ha scritto:
 
 
   ../daemon/event.c: In function 'virEventInterruptLocked':
   ../daemon/event.c:656: warning: cast from pointer to integer of
 different size [-Wpointer-to-int-cast]
 
   event.c: In function 'virEventInterruptLocked':
   event.c:656: warning: cast from pointer to integer of different size
 [-Wpointer-to-int-cast]
 
These two should be considered _more_ than simply warnings, they can
abort the software at runtime if used on 64-bit systems, so I'd suggest
tackling these _sooner_ than the shadows (I don't have compile-access to
my mac right now, or I'd be sending something myself).
 
-- 
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/

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

[libvirt] [PATCH] man pages: adjusted virsh help command to show updated options

2010-12-03 Thread Justin Clift

Updated to reflect that the optional --command parameter can
be given, as well as either a command name or group keyword.
---
 tools/virsh.pod |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9cb6829..a8f2912 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -115,7 +115,7 @@ The following commands are generic i.e. not specific to a 
domain.
 
 =over 4
 
-=item Bhelp optional Icommand-or-group
+=item Bhelp optional I--command Icommand-or-group-keyword
 
 This lists each of the virsh commands.  When used without options, all
 commands are listed, one per line, grouped into related categories,
-- 
1.7.3.2

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

Re: [libvirt] [PATCH] virsh: Remove redundant optional option for cmdHelp

2010-12-03 Thread Justin Clift

On 04/12/2010, at 4:12 AM, Justin Clift wrote:
 On 04/12/2010, at 2:24 AM, Eric Blake wrote:
 On 12/03/2010 12:34 AM, Osier Yang wrote:
 Remove the optional option group, as cmdHelp should accepts
 only one option, and rename option command as command-or-group,
 (virsh help supports both command and command group now, and user
 nealy uses the options, so it doesn't matter much for it being longer,
 :-)
 
 * tools/virsh.c
 ---
 tools/virsh.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)
 
 Given the ensuing discussion, I'm pushing with this squashed in.
 
 Thanks Eric, that works well.  I'll push a matching tweak to the man page in a
 minute too (under the trivial rule).

Actually, in this instance it's probably better if I submit it for ACKing 
rather than
just push it.  It should hit the list in a sec.

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

[libvirt] [PATCH] Implement virVMOperationType{To|From}String independent from WITH_MACVTAP

2010-12-03 Thread Matthias Bolte

As this symbols are exported independent from WITH_MACVTAP.
---
 src/util/macvtap.c |   33 +++--
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 2aa319c..4345d97 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -27,12 +27,13 @@
 
 #include config.h
 
+#include stdint.h
+
 #if WITH_MACVTAP || WITH_VIRTUALPORT
 
 # include stdio.h
 # include errno.h
 # include fcntl.h
-# include stdint.h
 # include c-ctype.h
 # include sys/socket.h
 # include sys/ioctl.h
@@ -44,10 +45,15 @@
 
 # include netlink/msg.h
 
-# include util.h
+#endif /* WITH_MACVTAP || WITH_VIRTUALPORT */
+
+#include util.h
+#include macvtap.h
+
+#if WITH_MACVTAP || WITH_VIRTUALPORT
+
 # include memory.h
 # include logging.h
-# include macvtap.h
 # include interface.h
 # include conf/domain_conf.h
 # include virterror_internal.h
@@ -77,17 +83,6 @@
 # define LLDPAD_PID_FILE  /var/run/lldpad.pid
 
 
-VIR_ENUM_IMPL(virVMOperation, VIR_VM_OP_LAST,
-create,
-save,
-restore,
-destroy,
-migrate out,
-migrate in start,
-migrate in finish,
-no-op)
-
-
 enum virVirtualPortOp {
 ASSOCIATE = 0x1,
 DISASSOCIATE = 0x2,
@@ -1609,3 +1604,13 @@ vpDisassociatePortProfileId(const char *macvtap_ifname,
 }
 
 #endif /* WITH_MACVTAP || WITH_VIRTUALPORT */
+
+VIR_ENUM_IMPL(virVMOperation, VIR_VM_OP_LAST,
+create,
+save,
+restore,
+destroy,
+migrate out,
+migrate in start,
+migrate in finish,
+no-op)
-- 
1.7.0.4

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

Re: [libvirt] [PATCH] build: avoid shadowing devname() on BSD systems

2010-12-03 Thread Justin Clift

On 04/12/2010, at 4:16 AM, Diego Elio Pettenò wrote:
 Il giorno sab, 04/12/2010 alle 04.08 +1100, Justin Clift ha scritto:
 
 
  ../daemon/event.c: In function 'virEventInterruptLocked':
  ../daemon/event.c:656: warning: cast from pointer to integer of
 different size [-Wpointer-to-int-cast]
 
  event.c: In function 'virEventInterruptLocked':
  event.c:656: warning: cast from pointer to integer of different size
 [-Wpointer-to-int-cast]
 
 These two should be considered _more_ than simply warnings, they can
 abort the software at runtime if used on 64-bit systems, so I'd suggest
 tackling these _sooner_ than the shadows (I don't have compile-access to
 my mac right now, or I'd be sending something myself).

Thanks.  They're from this piece of code:

if (!eventLoop.running ||
virThreadIsSelf(eventLoop.leader)) {
VIR_DEBUG(Skip interrupt, %d %d, eventLoop.running, 
(int)eventLoop.leader.thread);
return 0;
}

Like 656 is the VIR_DEBUG() line.

The eventLoop.running is defined earlier in the file as:

int running;

Having trouble finding where eventLoop.leader.thread is defined though.  
Probably because
I'm more sleepy than optimal.  Thinking it might be some kind of problem with 
OSX and gnulib?

Tracing things back manually shows this in the OSX provided 
/usr/include/pthread.h:

  #ifndef _PTHREAD_T
  #define _PTHREAD_T
  typedef __darwin_pthread_t  pthread_t;
  #endif

And this in the gnulib version, gnulib/lib/pthread.h:

 typedef int pthread_t;

Any idea if this on the right track, or am I just confusing myself? :)




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

Re: [libvirt] [PATCH] tests: Fix commandtest in VPATH build

2010-12-03 Thread Eric Blake

On 12/03/2010 09:21 AM, Jiri Denemark wrote:
 ---
  tests/commandtest.c |6 +-
  1 files changed, 5 insertions(+), 1 deletions(-)
 
 diff --git a/tests/commandtest.c b/tests/commandtest.c
 index ace2f33..31adeea 100644
 --- a/tests/commandtest.c
 +++ b/tests/commandtest.c
 @@ -509,9 +509,12 @@ cleanup:
   * Only stdin/out/err open
   */
  static int test15(const void *unused ATTRIBUTE_UNUSED) {
 +char *cwd = NULL;
  virCommandPtr cmd = virCommandNew(abs_builddir /commandhelper);
  
 -virCommandSetWorkingDirectory(cmd, abs_builddir /commanddata);
 +if (virAsprintf(cwd, %s/commanddata, abs_srcdir)  0)
 +return -1;

Oops - this needs to call virCommandFree(cmd) before returning.

 +virCommandSetWorkingDirectory(cmd, cwd);

Sweet - I knew it would be worth adding virCommandSetWorkingDirectory,
even when it wasn't in Dan's original implementation.

  
  if (virCommandRun(cmd, NULL)  0) {
  virErrorPtr err = virGetLastError();
 @@ -519,6 +522,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) {
  return -1;
  }
  
 +VIR_FREE(cwd);

Oops - this leaks cwd if you returned 3 lines earlier.  You can float
the VIR_FREE(cwd) up to just after the virCommandSetWorkingDirectory
call, since you don't need cwd after that point.

NACK - I'll need to see a leak-free v2 before approving.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] man pages: adjusted virsh help command to show updated options

2010-12-03 Thread Eric Blake

On 12/03/2010 10:23 AM, Justin Clift wrote:
 Updated to reflect that the optional --command parameter can
 be given, as well as either a command name or group keyword.
 ---
  tools/virsh.pod |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/tools/virsh.pod b/tools/virsh.pod
 index 9cb6829..a8f2912 100644
 --- a/tools/virsh.pod
 +++ b/tools/virsh.pod
 @@ -115,7 +115,7 @@ The following commands are generic i.e. not specific to a 
 domain.
  
  =over 4
  
 -=item Bhelp optional Icommand-or-group
 +=item Bhelp optional I--command Icommand-or-group-keyword

NACK.  We already have the practice of omitting the I--command
argument for a command where the string is recognized purely by position
(where the --option is optional).  For example, look at 'virsh suspend',
where domain is required (that is, where 'suspend --domain xyz' is
equivalent to 'suspend xyz'):

=item Bsuspend Idomain-id

Note that the virsh.pod is spelled domain-id, even though the optional
option is only spelled --domain, so we already have precedence for using
a longer, more-descriptive string in the man page than what you get for
'virsh help suspend'.

I don't think we need to make any further tweaks to virsh.pod for this
issue.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] Implement virVMOperationType{To|From}String independent from WITH_MACVTAP

2010-12-03 Thread Eric Blake

On 12/03/2010 10:43 AM, Matthias Bolte wrote:
 As this symbols are exported independent from WITH_MACVTAP.
 ---
  src/util/macvtap.c |   33 +++--
  1 files changed, 19 insertions(+), 14 deletions(-)

ACK.

Hmm, maybe we should patch ./autobuild.sh to intentionally try a
stripped-down build with as many features disabled as possible to help
catch some of these cases where a full-featured build passes but a
reduced build fails.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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] build: avoid shadowing devname() on BSD systems

2010-12-03 Thread Eric Blake

On 12/03/2010 10:08 AM, Justin Clift wrote:
 Thanks Eric.  Captured the complete make log this time, to ensure it's fixed.
 
 The same warning also appears in a few other places. :/
 
   libvirt.c: In function 'virDomainOpenConsole':
   libvirt.c:13169: warning: declaration of 'devname' shadows a global 
 declaration [-Wshadow]
   /usr/include/stdlib.h:290: warning: shadowed declaration is here [-Wshadow]

Yuck.  'devname' is just too handy to be penalized by BSD's namespace
pollution.  What if we instead do this in internal.h:

#include stdlib.h
/* Silence -Wshadow on BSD systems, which declare a devname() that we
 * don't care about */
#define devname vir_devname

then all other shadowing warnings should just go away, without us having
to worry about the problem cropping up again.

(I've seen this trick used in gnulib, where we got rid of shadowing
warnings for the poorly-named and now-obsolete index().)

 While looking for those, a few more warnings also showed up:
 
   ../daemon/event.c: In function 'virEventInterruptLocked':
   ../daemon/event.c:656: warning: cast from pointer to integer of different 
 size [-Wpointer-to-int-cast]
 
   virsh.c: In function 'vshReadlineInit':
   virsh.c:11575: warning: assignment discards qualifiers from pointer target 
 type
 
   event.c: In function 'virEventInterruptLocked':
   event.c:656: warning: cast from pointer to integer of different size 
 [-Wpointer-to-int-cast]

I agree with Diego - these are real bugs and need fixing.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 2/2] virCommand: document behavior on no output

2010-12-03 Thread Eric Blake

Option 2.  Which one should I go with?  By the way, my pending patch
for converting openvz to use virCommand instead of popen is impacted
(it has a potential null dereference if we go with option 1).

* docs/internals/command.html.in: Update documentation.
* src/util/command.c (virCommandSetOutputBuffer)
(virCommandSetErrorBuffer, virCommandProcessIO) Guarantee empty
string on no output.
---
 docs/internals/command.html.in |8 +---
 src/util/command.c |   14 ++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index ea9ec64..d2fb133 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -348,9 +348,11 @@
 /pre

 p
-  Once the command has finished executing, these buffers
-  will contain the output, or be NULL if there was no output. It
-  is the callers responsibility to free these buffers.
+  Once the command has finished executing, these buffers will
+  contain the output.  If there was no output but virCommandRun
+  was successful, then they will be an empty string; if there was
+  an error, then they might still be NULL. It is the callers
+  responsibility to free these buffers.
 /p

 h3a name=directorySetting working directory/a/h3
diff --git a/src/util/command.c b/src/util/command.c
index 1923799..4fb5048 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -860,6 +860,20 @@ virCommandProcessIO(virCommandPtr cmd)
 }
 }

+/* If there was no output, the populate with the empty string.  */
+if (cmd-outbuf  !*cmd-outbuf) {
+if ((*cmd-outbuf = strdup()) == NULL) {
+virReportOOMError();
+return -1;
+}
+}
+if (cmd-errbuf  !*cmd-errbuf) {
+if ((*cmd-errbuf = strdup()) == NULL) {
+virReportOOMError();
+return -1;
+}
+}
+
 return 0;
 }

-- 
1.7.3.2

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

[libvirt] [PATCH 1/2] virCommand: document behavior on no output

2010-12-03 Thread Eric Blake

Option 1: This patch (all callers have to worry about NULL buffers,
but checking for output is a simple pointer check).

Option 2: Guarantee that outbuf/errbuf are allocated, even if to
the empty string.  Caller always has to free the result, and
empty output check requires checking if *outbuf=='\0'.

Personally, I prefer option 2.  Thoughts?

* docs/internals/command.html.in: Update documentation.
* src/util/command.c (virCommandSetOutputBuffer)
(virCommandSetErrorBuffer) Guarantee NULL buffer on no output.
---
 docs/internals/command.html.in |4 ++--
 src/util/command.c |2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index c4fa800..ea9ec64 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -349,8 +349,8 @@

 p
   Once the command has finished executing, these buffers
-  will contain the output. It is the callers responsibility
-  to free these buffers.
+  will contain the output, or be NULL if there was no output. It
+  is the callers responsibility to free these buffers.
 /p

 h3a name=directorySetting working directory/a/h3
diff --git a/src/util/command.c b/src/util/command.c
index aa43f76..1923799 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -549,6 +549,7 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
 return;
 }

+VIR_FREE(*outbuf);
 cmd-outbuf = outbuf;
 cmd-outfdptr = cmd-outfd;
 }
@@ -569,6 +570,7 @@ virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf)
 return;
 }

+VIR_FREE(*errbuf);
 cmd-errbuf = errbuf;
 cmd-errfdptr = cmd-errfd;
 }
-- 
1.7.3.2

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

[libvirt] [PATCHv2 4/5] uuid: require smbios uuid and domain uuid to match

2010-12-03 Thread Eric Blake

* src/conf/domain_conf.c (virDomainDefParseXML): Prefer sysinfo
uuid over generating one, and if both uuids are present, require
them to be identical.
* src/qemu/qemu_conf.c (qemuBuildSmbiosSystemStr): Allow skipping
the uuid.
(qemudBuildCommandLine): Adjust caller; smbios mode=host/ must
not use host uuid in place of guest uuid.
---

The qemu developers pointed out that '-uuid xyz' and '-smbios
type=1,uuid=xyz' both have the same effect.  They argued that -uuid is
deprecated, but it's easier for libvirt to always supply it.
Meanwhile, they were clear that if both values were supplied, the two
uuids had better be identical (current qemu favors -uuid over -smbios,
but it's not guaranteed to stay that way in the future).

 src/conf/domain_conf.c |   22 +-
 src/qemu/qemu_conf.c   |   18 +++---
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3dfd4ee..9c54a59 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4498,6 +4498,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 long id = -1;
 virDomainDefPtr def;
 unsigned long count;
+bool uuid_generated = false;

 if (VIR_ALLOC(def)  0) {
 virReportOOMError();
@@ -4529,7 +4530,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 goto error;
 }

-/* Extract domain uuid */
+/* Extract domain uuid. If both uuid and sysinfo/system/entry/uuid
+ * exist, they must match; and if only the latter exists, it can
+ * also serve as the uuid. */
 tmp = virXPathString(string(./uuid[1]), ctxt);
 if (!tmp) {
 if (virUUIDGenerate(def-uuid)) {
@@ -4537,6 +4540,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
  %s, _(Failed to generate UUID));
 goto error;
 }
+uuid_generated = true;
 } else {
 if (virUUIDParse(tmp, def-uuid)  0) {
 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -5279,6 +5283,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,

 if (def-sysinfo == NULL)
 goto error;
+if (def-sysinfo-system_uuid != NULL) {
+unsigned char uuidbuf[VIR_UUID_BUFLEN];
+if (virUUIDParse(def-sysinfo-system_uuid, uuidbuf)  0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(malformed uuid element));
+goto error;
+}
+if (uuid_generated)
+memcpy(def-uuid, uuidbuf, VIR_UUID_BUFLEN);
+else if (memcmp(def-uuid, uuidbuf, VIR_UUID_BUFLEN) != 0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(UUID mismatch between uuid and 
+   sysinfo));
+goto error;
+}
+}
 }
 tmp = virXPathString(string(./os/smbios/@mode), ctxt);
 if (tmp) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 9a9d924..9ca1dad 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -3645,15 +3645,15 @@ error:
 return(NULL);
 }

-static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def)
+static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def, bool skip_uuid)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;

 if ((def-system_manufacturer == NULL)  (def-system_sku == NULL) 
-(def-system_product == NULL)  (def-system_uuid == NULL) 
-(def-system_version == NULL)  (def-system_serial == NULL) 
-(def-system_family == NULL))
-return(NULL);
+(def-system_product == NULL)  (def-system_version == NULL) 
+(def-system_serial == NULL)  (def-system_family == NULL) 
+(def-system_uuid == NULL || skip_uuid))
+return NULL;

 virBufferAddLit(buf, type=1);

@@ -3671,7 +3671,7 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr 
def)
 if (def-system_serial)
 virBufferVSprintf(buf, ,serial=%s, def-system_serial);
 /* 1:UUID */
-if (def-system_uuid)
+if (def-system_uuid  !skip_uuid)
 virBufferVSprintf(buf, ,uuid=%s, def-system_uuid);
 /* 1:SKU Number */
 if (def-system_sku)
@@ -4173,6 +4173,7 @@ qemudBuildCommandLine(virConnectPtr conn,
 if ((def-os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) 
 (def-os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) {
 virSysinfoDefPtr source = NULL;
+bool skip_uuid = false;

 if (!(qemuCmdFlags  QEMUD_CMD_FLAG_SMBIOS_TYPE)) {
 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -4189,6 +4190,8 @@ qemudBuildCommandLine(virConnectPtr conn,
 goto error;
 }
 source = driver-hostsysinfo;
+/* Host and guest uuid must differ, by definition of UUID. */
+skip_uuid = true;
 } else if (def-os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO)

[libvirt] [PATCHv2 2/5] smbios: support system family

2010-12-03 Thread Eric Blake

* docs/schemas/domain.rng (sysinfo-system-name): Also allow
family.
* src/util/sysinfo.h (struct _virSysinfoDef): Add system_family.
* src/conf/domain_conf.c (virSysinfoParseXML)
(virDomainSysinfoDefFormat): Support it.
* src/util/sysinfo.c (virSysinfoDefFree, virSysinfoRead): Likewise.
* src/qemu/qemu_conf.c (qemuBuildSmbiosSystemStr): Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-smbios.xml: Adjust test.
* tests/qemuxml2argvdata/qemuxml2argv-smbios.args: Likewise.
---
 docs/schemas/domain.rng |1 +
 src/conf/domain_conf.c  |9 -
 src/qemu/qemu_conf.c|6 +-
 src/util/sysinfo.c  |7 +++
 src/util/sysinfo.h  |1 +
 tests/qemuxml2argvdata/qemuxml2argv-smbios.args |2 +-
 tests/qemuxml2argvdata/qemuxml2argv-smbios.xml  |2 ++
 7 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 08ebefb..3dd7aa3 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1875,6 +1875,7 @@
   valueserial/value
   valueuuid/value
   valuesku/value
+  valuefamily/value
 /choice
   /define

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9516427..3dfd4ee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3625,6 +3625,8 @@ virSysinfoParseXML(const xmlNodePtr node,
 virXPathString(string(system/ent...@name='uuid']), ctxt);
 def-system_sku =
 virXPathString(string(system/ent...@name='sku']), ctxt);
+def-system_family =
+virXPathString(string(system/ent...@name='family']), ctxt);

 cleanup:
 VIR_FREE(type);
@@ -6425,7 +6427,8 @@ virDomainSysinfoDefFormat(virBufferPtr buf,
 }
 if ((def-system_manufacturer != NULL) || (def-system_product != NULL) ||
 (def-system_version != NULL) || (def-system_serial != NULL) ||
-(def-system_uuid != NULL) || (def-system_sku != NULL)) {
+(def-system_uuid != NULL) || (def-system_sku != NULL) ||
+(def-system_family != NULL)) {
 virBufferAddLit(buf, system\n);
 if (def-system_manufacturer != NULL)
 virBufferEscapeString(buf,
@@ -6451,6 +6454,10 @@ virDomainSysinfoDefFormat(virBufferPtr buf,
 virBufferEscapeString(buf,
 entry name='sku'%s/entry\n,
   def-system_sku);
+if (def-system_family != NULL)
+virBufferEscapeString(buf,
+entry name='family'%s/entry\n,
+  def-system_family);
 virBufferAddLit(buf, /system\n);
 }

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8985241..9a9d924 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -3651,7 +3651,8 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr 
def)

 if ((def-system_manufacturer == NULL)  (def-system_sku == NULL) 
 (def-system_product == NULL)  (def-system_uuid == NULL) 
-(def-system_version == NULL)  (def-system_serial == NULL))
+(def-system_version == NULL)  (def-system_serial == NULL) 
+(def-system_family == NULL))
 return(NULL);

 virBufferAddLit(buf, type=1);
@@ -3675,6 +3676,9 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr 
def)
 /* 1:SKU Number */
 if (def-system_sku)
 virBufferVSprintf(buf, ,sku=%s, def-system_sku);
+/* 1:Family */
+if (def-system_family)
+virBufferVSprintf(buf, ,family=%s, def-system_family);

 if (virBufferError(buf)) {
 virReportOOMError();
diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index 8ad98fe..cf41773 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -68,6 +68,7 @@ void virSysinfoDefFree(virSysinfoDefPtr def)
 VIR_FREE(def-system_serial);
 VIR_FREE(def-system_uuid);
 VIR_FREE(def-system_sku);
+VIR_FREE(def-system_family);
 VIR_FREE(def);
 }

@@ -217,6 +218,12 @@ virSysinfoRead(void) {
 if ((eol)  ((ret-system_sku = strndup(cur, eol - cur)) == NULL))
 goto no_memory;
 }
+if ((cur = strstr(base, Family: )) != NULL) {
+cur += 8;
+eol = strchr(cur, '\n');
+if ((eol)  ((ret-system_family = strndup(cur, eol - cur)) == NULL))
+goto no_memory;
+}

 cleanup:
 VIR_FREE(outbuf);
diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h
index 611d54e..1af7ef6 100644
--- a/src/util/sysinfo.h
+++ b/src/util/sysinfo.h
@@ -49,6 +49,7 @@ struct _virSysinfoDef {
 char *system_serial;
 char *system_uuid;
 char *system_sku;
+char *system_family;
 };

 virSysinfoDefPtr virSysinfoRead(void);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args 
b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
index bd3ede4..b5e4783 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args

[libvirt] [PATCHv2 1/5] qemu: avoid adding in smbios arguments

2010-12-03 Thread Eric Blake

The log lists things like -smbios type=1,vendor=Red Hat, which
is great for shell parsing, but not so great when you realize that
execve() then passes those literal  on as part of the command
line argument, such that qemu sets SMBIOS with extra literal quotes.

The eventual addition of virCommand is needed before we have the API
to shell-quote a string representation of a command line, so that the
log can still be pasted into a shell, but without inserting extra
bytes into the execve() arguments.

* src/qemu/qemu_conf.c (qemuBuildSmbiosBiosStr)
(qemuBuildSmbiosSystemStr): Qemu doesn't like quotes around uuid
arguments, and the remaining quotes are passed literally to
smbios, making smbios mode='host'/ inaccurate.  Removing the
quotes makes the log harder to parse, but that can be fixed later
with virCommand improvements.
* tests/qemuxml2argvdata/qemuxml2argv-smbios.args: 'Fix' test; it
will need fixing again once virCommand learns how to shell-quote a
potential command line.
---
 src/qemu/qemu_conf.c|   20 ++--
 tests/qemuxml2argvdata/qemuxml2argv-smbios.args |2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 925585a..8985241 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -3622,16 +3622,16 @@ static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr 
def)

 /* 0:Vendor */
 if (def-bios_vendor)
-virBufferVSprintf(buf, ,vendor=\%s\, def-bios_vendor);
+virBufferVSprintf(buf, ,vendor=%s, def-bios_vendor);
 /* 0:BIOS Version */
 if (def-bios_version)
-virBufferVSprintf(buf, ,version=\%s\, def-bios_version);
+virBufferVSprintf(buf, ,version=%s, def-bios_version);
 /* 0:BIOS Release Date */
 if (def-bios_date)
-virBufferVSprintf(buf, ,date=\%s\, def-bios_date);
+virBufferVSprintf(buf, ,date=%s, def-bios_date);
 /* 0:System BIOS Major Release and 0:System BIOS Minor Release */
 if (def-bios_release)
-virBufferVSprintf(buf, ,release=\%s\, def-bios_release);
+virBufferVSprintf(buf, ,release=%s, def-bios_release);

 if (virBufferError(buf)) {
 virReportOOMError();
@@ -3658,23 +3658,23 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr 
def)

 /* 1:Manufacturer */
 if (def-system_manufacturer)
-virBufferVSprintf(buf, ,manufacturer=\%s\,
+virBufferVSprintf(buf, ,manufacturer=%s,
   def-system_manufacturer);
  /* 1:Product Name */
 if (def-system_product)
-virBufferVSprintf(buf, ,product=\%s\, def-system_product);
+virBufferVSprintf(buf, ,product=%s, def-system_product);
 /* 1:Version */
 if (def-system_version)
-virBufferVSprintf(buf, ,version=\%s\, def-system_version);
+virBufferVSprintf(buf, ,version=%s, def-system_version);
 /* 1:Serial Number */
 if (def-system_serial)
-virBufferVSprintf(buf, ,serial=\%s\, def-system_serial);
+virBufferVSprintf(buf, ,serial=%s, def-system_serial);
 /* 1:UUID */
 if (def-system_uuid)
-virBufferVSprintf(buf, ,uuid=\%s\, def-system_uuid);
+virBufferVSprintf(buf, ,uuid=%s, def-system_uuid);
 /* 1:SKU Number */
 if (def-system_sku)
-virBufferVSprintf(buf, ,sku=\%s\, def-system_sku);
+virBufferVSprintf(buf, ,sku=%s, def-system_sku);

 if (virBufferError(buf)) {
 virReportOOMError();
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args 
b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
index d5bd289..bd3ede4 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
@@ -1 +1 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -smbios 
type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,serial=32dfcb37-5af1-552b-357c-be8c3aa38310,uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda 
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -smbios 
type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,serial=32dfcb37-5af1-552b-357c-be8c3aa38310,uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda 
/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb
-- 
1.7.3.2

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

[libvirt] [PATCHv2 0/5] smbios cleanups

2010-12-03 Thread Eric Blake

No change to the first three patches (but reposting so they'll all be
in the same thread).  The last two patches are new.

Eric Blake (5):
  qemu: avoid adding  in smbios arguments
  smbios: support system family
  smbios: allow () in smbios strings
  uuid: require smbios uuid and domain uuid to match
  sysinfo: convert to virCommand

 docs/schemas/domain.rng |3 +-
 src/conf/domain_conf.c  |   31 -
 src/qemu/qemu_conf.c|   40 ++--
 src/util/sysinfo.c  |   57 +++
 src/util/sysinfo.h  |1 +
 tests/qemuxml2argvdata/qemuxml2argv-smbios.args |2 +-
 tests/qemuxml2argvdata/qemuxml2argv-smbios.xml  |   20 
 7 files changed, 85 insertions(+), 69 deletions(-)

-- 
1.7.3.2

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

[libvirt] [PATCHv2 5/5] sysinfo: convert to virCommand

2010-12-03 Thread Eric Blake

* src/util/sysinfo.c (virSysinfoRead): Use virCommand instead of
virExec.
---

This patch assumes option 2 of my patch to virCommand guarantees on
outbuf when the child process had no output (it needs a slight tweak
to avoid a NULL dereference if we go with option 1 for virCommand).

 src/util/sysinfo.c |   50 ++
 1 files changed, 10 insertions(+), 40 deletions(-)

diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c
index cf41773..f44b112 100644
--- a/src/util/sysinfo.c
+++ b/src/util/sysinfo.c
@@ -36,6 +36,7 @@
 #include conf/domain_conf.h
 #include logging.h
 #include memory.h
+#include command.h

 #define VIR_FROM_THIS VIR_FROM_SYSINFO

@@ -94,25 +95,24 @@ virSysinfoRead(void) {
 virSysinfoDefPtr
 virSysinfoRead(void) {
 char *path, *cur, *eol, *base;
-int pid, outfd = -1, errfd = -1;
 virSysinfoDefPtr ret = NULL;
 const char *argv[] = { NULL, -q, -t, 0,1, NULL };
-int res, waitret, exitstatus;
+int res;
 char *outbuf = NULL;
-char *errbuf = NULL;
+virCommandPtr cmd;

 path = virFindFileInPath(SYSINFO_SMBIOS_DECODER);
 if (path == NULL) {
 virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to find path for %s binary),
  SYSINFO_SMBIOS_DECODER);
-return(NULL);
+return NULL;
 }
-argv[0] = path;

-res = virExec(argv, NULL, NULL, pid, -1, outfd, errfd,
-  VIR_EXEC_NONE | VIR_EXEC_NONBLOCK);
-if (res  0) {
+argv[0] = path;
+cmd = virCommandNewArgs(argv);
+virCommandSetOutputBuffer(cmd, outbuf);
+if (virCommandRun(cmd, NULL)  0) {
 virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to execute command %s),
  path);
@@ -120,34 +120,6 @@ virSysinfoRead(void) {
 goto cleanup;
 }

-/*
- * we are interested in the output, capture it and errors too
- */
-if (virPipeReadUntilEOF(outfd, errfd, outbuf, errbuf)  0) {
-virReportSystemError(errno, _(cannot wait for '%s'), path);
-while (waitpid(pid, exitstatus, 0) == -1  errno == EINTR)
-;
-goto cleanup;
-}
-
-if (outbuf)
-VIR_DEBUG(Command stdout: %s, outbuf);
-if (errbuf)
-VIR_DEBUG(Command stderr: %s, errbuf);
-
-while ((waitret = waitpid(pid, exitstatus, 0) == -1) 
-   (errno == EINTR));
-if (waitret == -1) {
-virReportSystemError(errno, _(Failed to wait for '%s'), path);
-goto cleanup;
-}
-if (exitstatus != 0) {
-virSmbiosReportError(VIR_ERR_INTERNAL_ERROR,
-   _(command %s failed with error code %d:%s),
- path, exitstatus, errbuf);
-goto cleanup;
-}
-
 if (VIR_ALLOC(ret)  0)
 goto no_memory;

@@ -227,15 +199,13 @@ virSysinfoRead(void) {

 cleanup:
 VIR_FREE(outbuf);
-VIR_FREE(errbuf);
-VIR_FREE(path);
+virCommandFree(cmd);

-return(ret);
+return ret;

 no_memory:
 virReportOOMError();

-
 virSysinfoDefFree(ret);
 ret = NULL;
 goto cleanup;
-- 
1.7.3.2

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

[libvirt] [PATCHv2 3/5] smbios: allow () in smbios strings

2010-12-03 Thread Eric Blake

* docs/schemas/domain.rng (sysinf-value): Expand pattern.
* tests/qemuxml2argvdata/qemuxml2argv-smbios.xml: Prefer '' over
 for attribute values.  Copy real hardware values.
* tests/qemuxml2argvdata/qemuxml2argv-smbios.args: Likewise.
---
 docs/schemas/domain.rng |2 +-
 tests/qemuxml2argvdata/qemuxml2argv-smbios.args |2 +-
 tests/qemuxml2argvdata/qemuxml2argv-smbios.xml  |   22 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 3dd7aa3..811d559 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1881,7 +1881,7 @@

   define name=sysinfo-value
 data type=string
-  param name='pattern'[a-zA-Z0-9/\-_\. ]+/param
+  param name='pattern'[a-zA-Z0-9/\-_\. \(\)]+/param
 /data
   /define

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args 
b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
index b5e4783..12ef04f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.args
@@ -1 +1 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -smbios type=0,vendor=QEmu/KVM,version=0.13 -smbios 
type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,serial=32dfcb37-5af1-552b-357c-be8c3aa38310,uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red
 Hat -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c 
-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M 
pc -m 214 -smp 1 -smbios type=0,vendor=LENOVO,version=6FET82WW (3.12 ) -smbios 
type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,serial=32dfcb37-5af1-552b-357c-be8c3aa38310,uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red
 Hat -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c 
-hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml
index 45b6dba..23ec1a7 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-smbios.xml
@@ -4,25 +4,25 @@
   memory219200/memory
   currentMemory219200/currentMemory
   vcpu1/vcpu
-  sysinfo type=smbios
+  sysinfo type='smbios'
 bios
-  entry name=vendorQEmu/KVM/entry
-  entry name=version0.13/entry
+  entry name='vendor'LENOVO/entry
+  entry name='version'6FET82WW (3.12 )/entry
 /bios
 system
-  entry name=manufacturerFedora/entry
-  entry name=productVirt-Manager/entry
-  entry name=version0.8.2-3.fc14/entry
-  entry name=serial32dfcb37-5af1-552b-357c-be8c3aa38310/entry
-  entry name=uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/entry
-  entry name=sku1234567890/entry
-  entry name=familyRed Hat/entry
+  entry name='manufacturer'Fedora/entry
+  entry name='product'Virt-Manager/entry
+  entry name='version'0.8.2-3.fc14/entry
+  entry name='serial'32dfcb37-5af1-552b-357c-be8c3aa38310/entry
+  entry name='uuid'c7a5fdbd-edaf-9455-926a-d65c16db1809/entry
+  entry name='sku'1234567890/entry
+  entry name='family'Red Hat/entry
 /system
   /sysinfo
   os
 type arch='i686' machine='pc'hvm/type
 boot dev='hd'/
-smbios mode=sysinfo/
+smbios mode='sysinfo'/
   /os
   clock offset='utc'/
   on_poweroffdestroy/on_poweroff
-- 
1.7.3.2

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

[libvirt] [PATCH] virExec: avoid undefined behavior

2010-12-03 Thread Eric Blake

* src/util/util.c (__virExec): Don't use FD_ISSET on out-of-bounds fd.
---

Noticed this one by inspection, while investigating
https://bugzilla.redhat.com/show_bug.cgi?id=659855

Don't know if it's the root cause of the crash in that bug, though.

 src/util/util.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/src/util/util.c b/src/util/util.c
index 79ca5d3..1b5bc68 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -570,8 +570,7 @@ __virExec(const char *const*argv,
 i != null 
 i != childout 
 i != childerr 
-(!keepfd ||
- !FD_ISSET(i, keepfd))) {
+(!keepfd || (i  FD_SETSIZE  !FD_ISSET(i, keepfd {
 tmpfd = i;
 VIR_FORCE_CLOSE(tmpfd);
 }
-- 
1.7.3.2

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

Re: [libvirt] [PATCH 0/2] smbios fixes

2010-12-03 Thread Eric Blake

On 12/03/2010 02:50 AM, Gleb Natapov wrote:
 I am surprised that libvirt still uses -uuid.

Why? It still works, and it's backwards compatible even to older qemu
where -smbios wasn't supported.  It's actually harder to omit -uuid and
use only -smbios, after first guaranteeing that -smbios is supported,
than it is blindly use -uuid.  And since -uuid may have more
implications than just the -smbios table, whereas -smbios should not
affect anything except the smbios table, it makes sense to continue to
use -uuid for any of those other implications.


 SMBIOS tables are only setup on x86 QEMU binaries, doing fullvirt.
 On non-x86, or QEMU used for Xen paravirt, the -uuid arg value
 would be used in other ways unrelated to SMBIOS. Thus replacing
 -uuid $UUID with -smbiios type=1,uuid=$UUID would be wrong.

 What non-x86 platforms libvirt supports? With Xen use -uuid or whatever
 they support. With KVM use only smbios type=1,uuid=$UUID. There is not
 valid reason that I see to use both. But if you use both of them for some
 strange reason please make sure you provide the same value for both.

 libvirt aims to support any and all QEMU target architectures
 not merely x86. There's no benefit to using -smbios type=1,uuid=$UUID
 with KVM, over -uuid $UUID.
 So why do you use them both? Even more interesting question why do you
 use them both with different $UUID? If you do, do not complain that there
 should be some kind of order between them.

There's the real question.  We use both as of libvirt 0.8.6 because we
added the new XML for sysinfo type='smbios'.../sysinfoossmbios
mode='sysinfo'//os, where sysinfo provides the argument to pass to
qemu's -smbios argument.

However, I agree with your assessment that -uuid and -smbios
type=1,uuid= should never differ, so I've proposed an updated patch
series to guarantee that behavior (particularly patch 4/5):
https://www.redhat.com/archives/libvir-list/2010-December/msg00237.html

As for whether it is useful to pass the host UUID to the client through
smbios, I agree with you that 1. it is still up in the air as to whether
that is a decent design, or whether something better should be dreamed
up, and 2. it would have to be done through smbios block 11 (OEM
strings) and NOT by changing the guest's uuid (whether that uuid was
supplied by uuid, by sysinfo, or identically in both locations, from
the libvirt XML).  But both of those points are part of the bigger
picture, and shouldn't affect whether my patch series is accepted for
fixing up current bugs in libvirt smbios handling (although it may imply
future libvirt patches to start creating an appropriate binary file for
use with -smbios binary=file for passing the host uuid as an oem string).

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
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 1/2] virCommand: document behavior on no output

2010-12-03 Thread Justin Clift

On 04/12/2010, at 8:28 AM, Eric Blake wrote:
 Option 1: This patch (all callers have to worry about NULL buffers,
 but checking for output is a simple pointer check).
 
 Option 2: Guarantee that outbuf/errbuf are allocated, even if to
 the empty string.  Caller always has to free the result, and
 empty output check requires checking if *outbuf=='\0'.
 
 Personally, I prefer option 2.  Thoughts?

2 seems safer.

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

52 matches

Mail list logo