Re: [libvirt] Coverity scan

2013-01-03 Thread Michal Privoznik
On 02.01.2013 20:22, John Ferlan wrote:
 On 12/20/2012 04:24 PM, Eric Blake wrote:
 On 12/20/2012 08:25 AM, John Ferlan wrote:

 First allow me to introduce myself - I'm John Ferlan a new Red Hat employee 
 (3 weeks).  I came from the closed world at HP where for the last 7 years I 
 worked in a group developing/supporting HP's Integrity Virtual Machine 
 software prior to it being outsourced to India this past May. I primarily 
 worked in the CLI/API and daemon space, although I also spent quite a bit 
 of time in the lower virtualization layers which mimicked the Integrity 
 instructions. I am very happy to be in the open world and look forward to 
 contributing.  Everyone has to start some where.

 Welcome to libvirt!  Based on recent list traffic, there are several
 people, not just John, and not just at Red Hat, that are aiming to
 provide initial contributions.  Be sure to provide your feedback and
 questions on the list, and hopefully we can use that to make our hacking
 documentation even easier to follow for future newcomers.

 You may want to figure out why your mailer doesn't automatically wrap
 long lines.  Webmail interfaces (gmail, zimbra) and Microsoft Exchange
 tend to be the worst mail agents when it comes to RFC compliance and
 lack of knobs to tune for improving the situation, which in turn can
 make it harder to read your messages and apply patches you submit.  You
 may notice that many people on this list use mutt or Thunderbird rather
 than web mailers, if only to have better formatted mail, but it's not a
 hard pre-requisite (we won't reject a patch just because of how it was
 sent, although it might take longer to apply if we have to jump through
 hoops to get it extracted from the mail).  For mailers that have the
 right knobs, turning on format=flowed is one way of sending reasonably
 formatted mail [but be aware that Thunderbird has had a rather nasty bug
 for several years now where defaulting to format=flowed can result in
 botching the whitespace of the quoted portion of a message you are
 replying to].


 My first task here at Red Hat was to triage a Coverity scan executed 
 against libvirt-1.0.0-1.fc19.src.rpm done in late November.  There were 285 
 issues documented. I quickly found that some of the defects found there 
 were already fixed in later submits upstream, so I ran a new Coverity scan 
 last Friday and it came back with 297 issues broken down as follows:

  1 ARRAY_VS_SINGLETON
 33 BAD_SIZEOF
 17 CHECKED_RETURN
  1 CONSTANT_EXPRESSION_RESULT
  5 COPY_PASTE_ERROR
 13 DEADCODE
 46 FORWARD_NULL
  2 MISSING_RETURN
  2 NEGATIVE_RETURNS
  7 NULL_RETURNS
  1 OVERRUN
137 RESOURCE_LEAK
 18 REVERSE_INULL
  1 SIGN_EXTENSION
  3 UNINIT
  8 UNUSED_VALUE
  2 USE_AFTER_FREE

 Thanks for doing this.  Helping to reduce the false positives and
 eliminate the real bugs will make it easier to run Coverity on a
 periodic basis and check for recent regressions while the code is still
 fresh on our minds.  Looking forward to the patches you come up with.

 
 
 Ran a new scan recently - the number of defects increased slightly (+4
 FORWARD_NULL, +2 MISSING_RETURN, and + 1 UNINIT).
 
 I worked my way through the entire list and just figured I'd start with
 an easier group for my first submit attempt. I have a series of
 relatively easy changes for the CHECKED_RETURN condition; however, I
 was hoping to perhaps get some feedback on two files which had more
 ramifications from just checking the return status.
 
 The first is 'src/phyp/phyp_driver.c'. Neither waitsocket() nor any of
 the callers check the return status. The implication being if select()
 fails the code will continue to wait. So is this desired? Conversely
 what are the ramifications of checking status on select() and using
 virReportSystemError()? Or perhaps some other way to log the failure?
 For any of the callers, any concerns over checking status return == -1
 and jumping to the err label? I guess I'm concerned over making
 logic/flow changes to some long standing assumption.  I've been on the
 opposite end of debugging one of those.

I think we should check for the return value of select() / waitsocket().
But I also think we can silently ignore EINTR. So the code should look
something like this:

errno = 0;
while (some_libssh2-ish_rubbish) {
if (waitsocket(...)  0  errno != EINTR) {
virReportSystemError(errno, %s, _(unable to wait on libssh2 socket);
goto err;
}
}

 
 The second is 'src/rpc/virnetsocket.c'.  The Coverity complaint is that
 the setsockopt() call to set SO_REUSEADDR doesn't check the return
 status. I think this particular case may be a cut-n-paste type change as
 the from virNetSocketNewListenTCP(). Unless I'm missing something, does
 setting the reuse addr on a connect socket do anything? Can it just
 safely be removed?

In fact on Linux (yet another difference from *BSD) it does make sense to set
SO_REUSEADDR 

[libvirt] Greetings and next release

2013-01-03 Thread Daniel Veillard
   Hello everybody,

  Happy new year and best wishes for 2013 !

When we planned the 1.0.1 release we took the option of taking a bit
of extra time to push mid-december and planning the following one for
the end of January. Based on this I suggest to enter the freeze for
libvirt-1.0.2 on the Thursday 24 so that we have a week to push for a
final release on the end of the month. We are already over 110 commits
since 1.0.1 so 3 weeks from now I would expect enough changes for a
interesting release :-)

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] [PATCH] qemu: Don't parse log output when starting up a domain

2013-01-03 Thread Michal Privoznik
On 02.01.2013 18:45, Eric Blake wrote:
 On 01/02/2013 08:23 AM, Michal Privoznik wrote:
 Despite our great effort we still parsed qemu log output.
 We wouldn't notice unless qemu changed the format of the
 logs slightly.
 
 Maybe mention that the upcoming qemu 1.4 does just that.
 
 Anyway, now we should gather all interesting
 knobs like pty paths from monitor. Moreover, since for
 historical reasons the first console can be just an alias to
 the first serial port, we need to check this and copy the
 pty path if that's the case to the first console.
 ---
  src/qemu/qemu_capabilities.c |  7 +++
  src/qemu/qemu_capabilities.h |  1 +
  src/qemu/qemu_process.c  | 20 ++--
  3 files changed, 26 insertions(+), 2 deletions(-)
 
 ACK, with a grammar nit.
 
 @@ -1544,8 +1545,23 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr 
 vm,
  if (qemuProcessLookupPTYs(vm-def-channels, vm-def-nchannels,
paths, chardevfmt)  0)
  return -1;
 +/* For historical reasons, console[0] can be just an alias
 + * for serial[0]; That's why we need to update it as well */
 
 Either you are starting a new sentence and need a full stop (serial[0].
 That's), or you don't need a capital after semicolon (serial[0];
 that's).  Also, s/well/well./ wouldn't hurt.
 

Fixed and pushed. Thanks.

Michal

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


[libvirt] [PATCH 0/4] Fix segfault while redefining snapshots and allow redefinition of external snapshots

2013-01-03 Thread Peter Krempa
This series fixes two issues while redefining snapshots:

1) Segfault when disk alignment of the new snapshot failed (patch 3)

2) Failure to redefine external snapshots. A check was missing to align disks 
for external snapshots. (patch 4)

Peter Krempa (4):
  snapshot: conf: Make virDomainSnapshotIsExternal more reusable
  snapshot: qemu: Separate logic blocks with newlines
  snapshot: qemu: Fix segfault and vanishing snapshots when redefining
  snapshot: qemu: Allow redefinition of external snapshots

 src/conf/snapshot_conf.c | 14 
 src/conf/snapshot_conf.h |  1 +
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_driver.c   | 57 +++-
 4 files changed, 54 insertions(+), 19 deletions(-)

-- 
1.8.0.2

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


[libvirt] [PATCH 4/4] snapshot: qemu: Allow redefinition of external snapshots

2013-01-03 Thread Peter Krempa
A redefinition of an external inactive snapshot/checkpoint wasn't
possible without this change.
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4c7558d..3a52b47 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11410,7 +11410,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,

 if (def-dom) {
 if (def-state == VIR_DOMAIN_DISK_SNAPSHOT ||
-def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+virDomainSnapshotDefIsExternal(def)) {
 align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 align_match = false;
 }
-- 
1.8.0.2

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


[libvirt] [PATCH 1/4] snapshot: conf: Make virDomainSnapshotIsExternal more reusable

2013-01-03 Thread Peter Krempa
Allow to use definition objects with this predicate function.
---
 src/conf/snapshot_conf.c | 14 ++
 src/conf/snapshot_conf.h |  1 +
 src/libvirt_private.syms |  1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 201c586..0c5b005 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1058,17 +1058,23 @@ cleanup:


 bool
-virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
+virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def)
 {
 int i;

-if (snap-def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+if (def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
 return true;

-for (i = 0; i  snap-def-ndisks; i++) {
-if (snap-def-disks[i].snapshot == 
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+for (i = 0; i  def-ndisks; i++) {
+if (def-disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
 return true;
 }

 return false;
 }
+
+bool
+virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
+{
+return virDomainSnapshotDefIsExternal(snap-def);
+}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index b0f8760..f1d5995 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -166,6 +166,7 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr 
snapshots,
virDomainSnapshotPtr **snaps,
unsigned int flags);

+bool virDomainSnapshotDefIsExternal(virDomainSnapshotDefPtr def);
 bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap);

 VIR_ENUM_DECL(virDomainSnapshotLocation)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 497d5d3..d063382 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1097,6 +1097,7 @@ virDomainSnapshotAlignDisks;
 virDomainSnapshotAssignDef;
 virDomainSnapshotDefFormat;
 virDomainSnapshotDefFree;
+virDomainSnapshotDefIsExternal;
 virDomainSnapshotDefParseString;
 virDomainSnapshotDropParent;
 virDomainSnapshotFindByName;
-- 
1.8.0.2

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


[libvirt] [PATCH 2/4] snapshot: qemu: Separate logic blocks with newlines

2013-01-03 Thread Peter Krempa
---
 src/qemu/qemu_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b8bb36b..0883a56 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11365,6 +11365,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 goto cleanup;

 }
+
 if (def-dom 
 memcmp(def-dom-uuid, domain-uuid, VIR_UUID_BUFLEN)) {
 virReportError(VIR_ERR_INVALID_ARG,
@@ -11372,6 +11373,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
def-name, uuidstr);
 goto cleanup;
 }
+
 other = virDomainSnapshotFindByName(vm-snapshots, def-name);
 if (other) {
 if ((other-def-state == VIR_DOMAIN_RUNNING ||
@@ -11384,6 +11386,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
def-name);
 goto cleanup;
 }
+
 if ((other-def-state == VIR_DOMAIN_DISK_SNAPSHOT) !=
 (def-state == VIR_DOMAIN_DISK_SNAPSHOT)) {
 virReportError(VIR_ERR_INVALID_ARG,
@@ -11392,6 +11395,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
def-name);
 goto cleanup;
 }
+
 if (other-def-dom) {
 if (def-dom) {
 if (!virDomainDefCheckABIStability(other-def-dom,
@@ -11403,10 +11407,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 other-def-dom = NULL;
 }
 }
+
 if (other == vm-current_snapshot) {
 update_current = true;
 vm-current_snapshot = NULL;
 }
+
 /* Drop and rebuild the parent relationship, but keep all
  * child relations by reusing snap.  */
 virDomainSnapshotDropParent(other);
-- 
1.8.0.2

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


[libvirt] [PATCH 3/4] snapshot: qemu: Fix segfault and vanishing snapshots when redefining

2013-01-03 Thread Peter Krempa
When the disk alignment check done while redefining an existing snapshot
failed, the qemu driver attempted to free the existing snapshot. As in
the cleanup path the definition of the snapshot wasn't assigned, the
cleanup code dereferenced a NULL pointer.

This patch changes the behavior on error paths while redefining snapshot
in two ways:

1) On failure, modifications done on the snapshot definiton object are
rolled back.

2) The previous definition of the data isn't freed until it's certain it
won't be needed any more.

This change avoids the segfault and additionaly the snapshot doesn't
vanish if re-definiton fails for some reason.
---
 src/qemu/qemu_driver.c | 51 +++---
 1 file changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0883a56..4c7558d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11408,6 +11408,24 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 }
 }

+if (def-dom) {
+if (def-state == VIR_DOMAIN_DISK_SNAPSHOT ||
+def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+align_match = false;
+}
+
+if (virDomainSnapshotAlignDisks(def, align_location,
+align_match)  0) {
+/* revert stealing of the snapshot domain definition */
+if (def-dom  !other-def-dom) {
+other-def-dom = def-dom;
+def-dom = NULL;
+}
+goto cleanup;
+}
+}
+
 if (other == vm-current_snapshot) {
 update_current = true;
 vm-current_snapshot = NULL;
@@ -11417,18 +11435,20 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
  * child relations by reusing snap.  */
 virDomainSnapshotDropParent(other);
 virDomainSnapshotDefFree(other-def);
-other-def = NULL;
+other-def = def;
+def = NULL;
 snap = other;
-}
-if (def-dom) {
-if (def-state == VIR_DOMAIN_DISK_SNAPSHOT ||
-def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
-align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-align_match = false;
+} else {
+if (def-dom) {
+if (def-state == VIR_DOMAIN_DISK_SNAPSHOT ||
+def-memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+align_match = false;
+}
+if (virDomainSnapshotAlignDisks(def, align_location,
+align_match)  0)
+goto cleanup;
 }
-if (virDomainSnapshotAlignDisks(def, align_location,
-align_match)  0)
-goto cleanup;
 }
 } else {
 /* Easiest way to clone inactive portion of vm-def is via
@@ -11463,11 +11483,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
 goto cleanup;
 }

-if (snap)
-snap-def = def;
-else if (!(snap = virDomainSnapshotAssignDef(vm-snapshots, def)))
-goto cleanup;
-def = NULL;
+if (!snap) {
+if (!(snap = virDomainSnapshotAssignDef(vm-snapshots, def)))
+goto cleanup;
+
+def = NULL;
+}

 if (update_current)
 snap-def-current = true;
-- 
1.8.0.2

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


[libvirt] [PATCH v2 5/8] virsh: Expose virDomainInterfacesAddresses

2013-01-03 Thread Michal Privoznik
---
 tools/virsh-domain.c | 104 +++
 1 file changed, 104 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f3da1d5..e6c09f0 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8491,6 +8491,109 @@ cleanup:
 return ret;
 }
 
+static const vshCmdInfo info_domifaddrs[] = {
+{help, N_(Dump domain's IP addresses and other interfaces info)},
+{desc, N_(Dump domain's IP addresses and other interfaces info)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_domifaddrs[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{interface, VSH_OT_STRING, 0, N_(Limit selection just to one 
interface)},
+{method, VSH_OT_STRING, 0, N_(Use one method: default, agent, 
nwfilter)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdDomIfAddrs(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+bool ret = false;
+unsigned int flags = 0;
+const char *device = NULL;
+const char *methodStr = NULL;
+int method = VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT;
+virDomainInterfacePtr *ifaces = NULL;
+int i, j, ifacesCount = 0;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+goto cleanup;
+
+if (vshCommandOptString(cmd, interface, device)  0) {
+vshError(ctl, _(Unable to parse interface parameter));
+goto cleanup;
+}
+
+if (vshCommandOptString(cmd, method, methodStr)  0) {
+vshError(ctl, _(Unable to parse method parameter));
+goto cleanup;
+}
+
+if (STREQ_NULLABLE(methodStr, default))
+method = VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT;
+else if (STREQ_NULLABLE(methodStr, agent))
+method = VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT;
+else if (STREQ_NULLABLE(methodStr, nwfilter))
+method = VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER;
+else if (methodStr) {
+vshError(ctl, _(Unknown method: %s), methodStr);
+goto cleanup;
+}
+
+ifacesCount = virDomainInterfacesAddresses(dom, ifaces, method, flags);
+if (ifacesCount  0)
+goto cleanup;
+
+vshPrintExtra(ctl,  %-10s %-17s%s\n%s\n,
+  _(Name), _(HW address), _(IP address),
+  ---);
+for (i = 0; i  ifacesCount; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+const char *hwaddr = ;
+const char *ipAddrStr = NULL;
+
+if (device  STRNEQ(device, iface-name))
+continue;
+
+if (iface-hwaddr)
+hwaddr = iface-hwaddr;
+
+for (j = 0; j  iface-ip_addrs_count; j++) {
+const char *type = ;
+if (iface-ip_addrs[j].type == VIR_IP_ADDR_TYPE_IPV4)
+type = inet ;
+else if (iface-ip_addrs[j].type == VIR_IP_ADDR_TYPE_IPV6)
+type = inet6 ;
+if (j)
+virBufferAddChar(buf, ' ');
+virBufferAsprintf(buf, %s%s/%d,
+  type,
+  iface-ip_addrs[j].addr,
+  iface-ip_addrs[j].prefix);
+}
+
+ipAddrStr = virBufferContentAndReset(buf);
+
+if (!ipAddrStr)
+ipAddrStr = vshStrdup(ctl,);
+
+vshPrintExtra(ctl,  %-10s %-17s%s\n,
+  iface-name, hwaddr, ipAddrStr);
+
+VIR_FREE(ipAddrStr);
+}
+
+ret = true;
+cleanup:
+for (i = 0; i  ifacesCount; i++)
+virDomainInterfaceFree(ifaces[i]);
+VIR_FREE(ifaces);
+if (dom)
+virDomainFree(dom);
+return ret;
+}
+
 const vshCmdDef domManagementCmds[] = {
 {attach-device, cmdAttachDevice, opts_attach_device,
  info_attach_device, 0},
@@ -8527,6 +8630,7 @@ const vshCmdDef domManagementCmds[] = {
 {domhostname, cmdDomHostname, opts_domhostname, info_domhostname, 0},
 {domid, cmdDomid, opts_domid, info_domid, 0},
 {domif-setlink, cmdDomIfSetLink, opts_domif_setlink, info_domif_setlink, 
0},
+{domifaddrs, cmdDomIfAddrs, opts_domifaddrs, info_domifaddrs, 0},
 {domiftune, cmdDomIftune, opts_domiftune, info_domiftune, 0},
 {domjobabort, cmdDomjobabort, opts_domjobabort, info_domjobabort, 0},
 {domjobinfo, cmdDomjobinfo, opts_domjobinfo, info_domjobinfo, 0},
-- 
1.8.0.2

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


[libvirt] [PATCH v2 4/8] qemu: Implement qemuDomainInterfacesAddresses

2013-01-03 Thread Michal Privoznik
---
 src/qemu/qemu_driver.c | 68 ++
 1 file changed, 68 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b8bb36b..662d956 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14520,6 +14520,73 @@ cleanup:
 return ret;
 }
 
+static int
+qemuDomainInterfacesAddresses(virDomainPtr dom,
+  virDomainInterfacePtr **ifaces,
+  unsigned int method,
+  unsigned int flags)
+{
+virQEMUDriverPtr driver = dom-conn-privateData;
+virDomainObjPtr vm;
+int ret = -1;
+qemuDomainObjPrivatePtr priv;
+
+virCheckFlags(0, -1);
+
+if (method != VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT 
+method != VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(Only guest agent method is supported for now));
+return -1;
+}
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+priv = vm-privateData;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(domain is not running));
+goto cleanup;
+}
+
+if (!priv-agent) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(QEMU guest agent is not configured));
+goto cleanup;
+}
+
+if (priv-agentError) {
+virReportError(VIR_ERR_AGENT_UNRESPONSIVE, %s,
+   _(QEMU guest agent is not 
+ available due to an error));
+goto cleanup;
+}
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY)  0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(domain is not running));
+goto endjob;
+}
+
+qemuDomainObjEnterAgent(driver, vm);
+ret = qemuAgentGetInterfaces(priv-agent, ifaces);
+qemuDomainObjExitAgent(driver, vm);
+
+endjob:
+if (qemuDomainObjEndJob(driver, vm) == 0)
+vm = NULL;
+
+cleanup:
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
 static virDriver qemuDriver = {
 .no = VIR_DRV_QEMU,
 .name = QEMU_DRIVER_NAME,
@@ -14694,6 +14761,7 @@ static virDriver qemuDriver = {
 .nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */
 .nodeGetCPUMap = nodeGetCPUMap, /* 1.0.0 */
 .domainFSTrim = qemuDomainFSTrim, /* 1.0.1 */
+.domainInterfacesAddresses = qemuDomainInterfacesAddresses, /* 1.0.2 */
 };
 
 
-- 
1.8.0.2

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


[libvirt] [PATCH v2 8/8] python: create example for dumping domain IP addresses

2013-01-03 Thread Michal Privoznik
---
 examples/python/Makefile.am   |  2 +-
 examples/python/README|  1 +
 examples/python/domipaddrs.py | 50 +++
 3 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100755 examples/python/domipaddrs.py

diff --git a/examples/python/Makefile.am b/examples/python/Makefile.am
index b04d126..b51f729 100644
--- a/examples/python/Makefile.am
+++ b/examples/python/Makefile.am
@@ -4,4 +4,4 @@
 EXTRA_DIST=\
README  \
consolecallback.py  \
-   dominfo.py domrestore.py domsave.py domstart.py esxlist.py
+   dominfo.py domrestore.py domsave.py domstart.py esxlist.py domipaddrs.py
diff --git a/examples/python/README b/examples/python/README
index f4db76c..d895740 100644
--- a/examples/python/README
+++ b/examples/python/README
@@ -10,6 +10,7 @@ domsave.py  - save all running domU's into a directory
 domrestore.py - restore domU's from their saved files in a directory
 esxlist.py  - list active domains of an VMware ESX host and print some info.
   also demonstrates how to use the libvirt.openAuth() method
+domipaddrs.py - print domain interfaces among with their HW and IP addresses
 
 The XML files in this directory are examples of the XML format that libvirt
 expects, and will have to be adapted for your setup. They are only needed
diff --git a/examples/python/domipaddrs.py b/examples/python/domipaddrs.py
new file mode 100755
index 000..82c5774
--- /dev/null
+++ b/examples/python/domipaddrs.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# domipaddrds - print IP addresses for running domain
+
+import libvirt
+import sys
+
+def usage():
+print Usage: %s [URI] DOMAIN % sys.argv[0]
+print Print IP addresses assigned to domain interfaces
+
+uri = None
+name = None
+args = len(sys.argv)
+
+if args == 2:
+name = sys.argv[1]
+elif args == 3:
+uri = sys.argv[1]
+name = sys.argv[2]
+else:
+usage()
+sys.exit(2)
+
+conn = libvirt.openReadOnly(uri)
+if conn == None:
+print Unable to open connection to libvirt
+sys.exit(1)
+
+try:
+dom = conn.lookupByName(name)
+except libvirt.libvirtError:
+print Domain %s not found % name
+sys.exit(0)
+
+ifaces = dom.interfacesAddresses(libvirt.VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT, 0)
+if (ifaces == None):
+print Failed to get domain interfaces
+sys.exit(0)
+
+print  {0:10} {1:17}{2}.format(Interface, HW address, IP Address)
+
+for (name, val) in ifaces.iteritems():
+print  {0:10} {1:17}.format(name, val['hwaddr']),
+
+if (val['ip_addrs']  None):
+print   ,
+for addr in val['ip_addrs']:
+print {0}/{1} .format(addr['addr'], addr['prefix']),
+
+print
-- 
1.8.0.2

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


[libvirt] [PATCH v2 6/8] remote: Implement virDomainInterfacesAddresses

2013-01-03 Thread Michal Privoznik
---
 daemon/remote.c  | 140 +++
 src/remote/remote_driver.c   |  94 +
 src/remote/remote_protocol.x |  27 -
 src/remote_protocol-structs  |  27 +
 4 files changed, 287 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 8767c18..7363e2a 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4783,3 +4783,143 @@ no_memory:
 virReportOOMError();
 return -1;
 }
+
+static int
+remoteSerializeDomainInterfacesPtr(virDomainInterfacePtr *ifaces,
+   int ifacesCount,
+   remote_domain_interfaces_addresses_ret *ret)
+{
+int i, j;
+
+if (!ifacesCount)
+return 0;
+
+if (VIR_ALLOC_N(ret-ifaces.ifaces_val, ifacesCount)  0) {
+virReportOOMError();
+return -1;
+}
+
+ret-ifaces.ifaces_len = ifacesCount;
+
+for (i = 0; i  ifacesCount; i++) {
+virDomainInterfacePtr tmp = ifaces[i];
+remote_domain_interface *tmp_ret = (ret-ifaces.ifaces_val[i]);
+
+if (!(tmp_ret-name = strdup(tmp-name)))
+goto no_memory;
+
+if (tmp-hwaddr) {
+char **hwaddr_p = NULL;
+if (VIR_ALLOC(hwaddr_p)  0)
+goto no_memory;
+*hwaddr_p = strdup(tmp-hwaddr);
+if (!*hwaddr_p) {
+VIR_FREE(hwaddr_p);
+goto no_memory;
+}
+
+tmp_ret-hwaddr = hwaddr_p;
+}
+
+tmp_ret-flags = tmp-flags;
+
+if (!tmp-ip_addrs_count)
+continue;
+
+if (VIR_ALLOC_N(tmp_ret-ip_addrs.ip_addrs_val,
+tmp-ip_addrs_count)  0)
+goto no_memory;
+
+tmp_ret-ip_addrs.ip_addrs_len = tmp-ip_addrs_count;
+
+for (j = 0; j  tmp-ip_addrs_count; j++) {
+virDomainIPAddress addr = tmp-ip_addrs[j];
+remote_domain_ip_addrs *addr_ret = 
(tmp_ret-ip_addrs.ip_addrs_val[j]);
+
+addr_ret-prefix = addr.prefix;
+addr_ret-type = addr.type;
+
+if (addr.addr) {
+char **addr_p = NULL;
+if (VIR_ALLOC(addr_p)  0)
+goto no_memory;
+*addr_p = strdup(addr.addr);
+if (!*addr_p) {
+VIR_FREE(addr_p);
+goto no_memory;
+}
+addr_ret-addr = addr_p;
+}
+
+if (addr.dstaddr) {
+char **addr_p = NULL;
+if (VIR_ALLOC(addr_p)  0)
+goto no_memory;
+*addr_p = strdup(addr.dstaddr);
+if (!*addr_p) {
+VIR_FREE(addr_p);
+goto no_memory;
+}
+addr_ret-dstaddr = addr_p;
+}
+}
+}
+
+return 0;
+
+no_memory:
+virReportOOMError();
+for (i = 0; i  ret-ifaces.ifaces_len; i++) {
+remote_domain_interface tmp_ret = ret-ifaces.ifaces_val[i];
+
+VIR_FREE(tmp_ret.name);
+VIR_FREE(tmp_ret.hwaddr);
+for (j = 0; j  tmp_ret.ip_addrs.ip_addrs_len; j++) {
+VIR_FREE(tmp_ret.ip_addrs.ip_addrs_val[j].addr);
+VIR_FREE(tmp_ret.ip_addrs.ip_addrs_val[j].dstaddr);
+}
+}
+VIR_FREE(ret-ifaces.ifaces_val);
+
+return -1;
+}
+
+static int
+remoteDispatchDomainInterfacesAddresses(virNetServerPtr server 
ATTRIBUTE_UNUSED,
+virNetServerClientPtr client,
+virNetMessagePtr msg ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+
remote_domain_interfaces_addresses_args *args,
+remote_domain_interfaces_addresses_ret 
*ret)
+{
+int rv = -1;
+virDomainPtr dom = NULL;
+virDomainInterfacePtr *ifaces = NULL;
+int ifacesCount = 0;
+struct daemonClientPrivate *priv = 
virNetServerClientGetPrivateData(client);
+
+if (!(dom = get_nonnull_domain(priv-conn, args-domain)))
+goto cleanup;
+
+ifacesCount = virDomainInterfacesAddresses(dom, ifaces,
+   args-method,
+   args-flags);
+if (ifacesCount  0)
+goto cleanup;
+
+if (remoteSerializeDomainInterfacesPtr(ifaces, ifacesCount, ret)  0)
+goto cleanup;
+
+rv = 0;
+
+cleanup:
+if (rv  0)
+virNetMessageSaveError(rerr);
+if (dom)
+virDomainFree(dom);
+
+while (ifacesCount  0)
+virDomainInterfaceFree(ifaces[--ifacesCount]);
+VIR_FREE(ifaces);
+return rv;
+}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ae861cc..7290f30 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5834,6 +5834,99 @@ done:
 return rv;
 }
 

[libvirt] [PATCH v2 3/8] qemu_agent: Implement 'guest-network-get-interfaces' command handling

2013-01-03 Thread Michal Privoznik
This command returns an array of all guest interfaces among
with their IP and HW addresses.
---
 src/qemu/qemu_agent.c | 171 ++
 src/qemu/qemu_agent.h |   2 +
 2 files changed, 173 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index bb421bd..409ba04 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1474,3 +1474,174 @@ qemuAgentFSTrim(qemuAgentPtr mon,
 virJSONValueFree(reply);
 return ret;
 }
+
+static int
+getInterfaces(virJSONValuePtr reply,
+  virDomainInterfacePtr **ifaces)
+{
+int ret = -1;
+int i, size = 0;
+virJSONValuePtr replyArray = NULL;
+
+if (!(replyArray = virJSONValueObjectGet(reply, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent did not provide 'return' object));
+goto cleanup;
+}
+
+if ((size = virJSONValueArraySize(replyArray))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent did not provide any interface));
+goto cleanup;
+}
+
+if (size  VIR_ALLOC_N(*ifaces, size * sizeof(virDomainInterfacePtr))  
0) {
+virReportOOMError();
+goto cleanup;
+}
+
+for (i = 0; i  size; i++) {
+virJSONValuePtr jsonIface = virJSONValueArrayGet(replyArray, i);
+virDomainInterfacePtr tmpIface = NULL;
+virJSONValuePtr jsonIpAddrArr = NULL;
+int j, jsonIpAddrArrSize = 0;
+const char *name, *hwaddr;
+
+if (VIR_ALLOC(tmpIface)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+(*ifaces)[i] = tmpIface;
+/* should not happen, but doesn't hurt to check */
+if (!jsonIface) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Something went really wrong while processing 
+ guest agent reply));
+goto cleanup;
+}
+
+name = virJSONValueObjectGetString(jsonIface, name);
+if (!name) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent did not provide 'name' object));
+goto cleanup;
+}
+
+if (!(tmpIface-name = strdup(name))) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* hwaddr might be omitted */
+hwaddr = virJSONValueObjectGetString(jsonIface, hardware-address);
+if (hwaddr  !(tmpIface-hwaddr = strdup(hwaddr))) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* as well as ip-addresses */
+jsonIpAddrArr = virJSONValueObjectGet(jsonIface, ip-addresses);
+if (!jsonIpAddrArr)
+continue;
+
+if ((jsonIpAddrArrSize = virJSONValueArraySize(jsonIpAddrArr))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent provided malformed 
+ ip-addresses field));
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(tmpIface-ip_addrs, jsonIpAddrArrSize)  0) {
+virReportOOMError();
+goto cleanup;
+}
+tmpIface-ip_addrs_count = jsonIpAddrArrSize;
+
+for (j = 0; j   jsonIpAddrArrSize; j++) {
+virJSONValuePtr jsonIpAddr = virJSONValueArrayGet(jsonIpAddrArr, 
j);
+virDomainIPAddressPtr tmpIpAddr = (tmpIface-ip_addrs[j]);
+const char *ipAddr, *ipAddrType;
+
+if (!(ipAddr = virJSONValueObjectGetString(jsonIpAddr,
+   ip-address))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu-agent didn't provided 
+ an ip-address field));
+goto cleanup;
+}
+
+if (!(tmpIpAddr-addr = strdup(ipAddr))) {
+virReportOOMError();
+goto cleanup;
+}
+
+if (!(ipAddrType = virJSONValueObjectGetString(jsonIpAddr,
+   
ip-address-type))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu-agent didn't provided 
+ an ip-address-type field));
+goto cleanup;
+}
+
+if (STREQ(ipAddrType, ipv4))
+tmpIpAddr-type = VIR_IP_ADDR_TYPE_IPV4;
+else if (STREQ(ipAddrType, ipv6))
+tmpIpAddr-type = VIR_IP_ADDR_TYPE_IPV6;
+else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(qemu agent provided unknown 
+ ip-address-type '%s'),
+   ipAddrType);
+goto cleanup;
+}
+
+if (virJSONValueObjectGetNumberInt(jsonIpAddr, prefix,
+  

[libvirt] [PATCH v2 7/8] python: Expose virDomainInterfacesAddresses

2013-01-03 Thread Michal Privoznik
---
 python/libvirt-override-api.xml |   7 +++
 python/libvirt-override.c   | 120 
 2 files changed, 127 insertions(+)

diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index a0e0496..bea1a59 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -548,5 +548,12 @@
   arg name='conn' type='virConnectPtr' info='pointer to the hypervisor 
connection'/
   arg name='flags' type='int' info='unused, pass 0'/
 /function
+function name='virDomainInterfacesAddresses' file='python'
+  infoGet domain's interfaces among with their IP and HW addresses/info
+  return type='virDomainInterfacePtr' info='array of @domain interfaces'/
+  arg name='domain' type='virDomainPtr' info='domain object'/
+  arg name='method' type='unsigned int' info='which method use, one of 
virDomainInterfacesAddressesMethod'/
+  arg name='flags' type='unsigned int' info='extra flags, not used yet, 
so callers should always pass 0'/
+/function
   /symbols
 /api
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index 91e82c6..c8365d4 100644
--- a/python/libvirt-override.c
+++ b/python/libvirt-override.c
@@ -6588,6 +6588,125 @@ error:
 goto cleanup;
 }
 
+static PyObject *
+libvirt_virDomainInterfacesAddresses(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args)
+{
+PyObject *py_retval;
+virDomainPtr domain;
+PyObject *pyobj_domain;
+unsigned int method, flags;
+virDomainInterfacePtr *ifaces = NULL;
+int j, i, i_retval = 0;
+bool full_free = true;
+
+if (!PyArg_ParseTuple(args, (char *) Oii:virDomainInterfacesAddresses,
+  pyobj_domain, method, flags))
+return NULL;
+
+domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+i_retval = virDomainInterfacesAddresses(domain, ifaces, method, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+if (i_retval  0) {
+py_retval = VIR_PY_NONE;
+goto cleanup;
+}
+
+if (!(py_retval = PyDict_New()))
+goto no_memory;
+
+for (i = 0; i  i_retval; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+PyObject *pyIPAddrs = NULL;
+PyObject *pyIface = NULL;
+
+if (!(pyIface = PyDict_New()))
+goto no_memory;
+
+if (iface-ip_addrs_count) {
+if (!(pyIPAddrs = PyList_New(iface-ip_addrs_count))) {
+Py_DECREF(pyIface);
+goto no_memory;
+}
+} else {
+pyIPAddrs = VIR_PY_NONE;
+}
+
+for (j = 0; j  iface-ip_addrs_count; j++) {
+virDomainIPAddress addr = iface-ip_addrs[j];
+PyObject *pyAddr = PyDict_New();
+const char *type = NULL;
+
+if (!pyAddr) {
+{ Py_DECREF(pyIface); }
+{ Py_DECREF(pyIPAddrs); }
+goto no_memory;
+}
+
+switch (addr.type) {
+case VIR_IP_ADDR_TYPE_IPV4:
+type = ipv4;
+break;
+case VIR_IP_ADDR_TYPE_IPV6:
+type = ipv6;
+break;
+default:
+type = unknown;
+break;
+}
+
+PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(addr),
+   PyString_FromString(addr.addr));
+PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(prefix),
+   libvirt_intWrap(addr.prefix));
+PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(type),
+   PyString_FromString(type));
+PyDict_SetItem(pyAddr, libvirt_constcharPtrWrap(dstaddr),
+   libvirt_charPtrWrap(addr.dstaddr));
+PyList_SetItem(pyIPAddrs, j, pyAddr);
+}
+
+PyDict_SetItem(pyIface, libvirt_constcharPtrWrap(ip_addrs),
+   pyIPAddrs);
+PyDict_SetItem(pyIface, libvirt_constcharPtrWrap(hwaddr),
+   libvirt_charPtrWrap(iface-hwaddr));
+PyDict_SetItem(pyIface, libvirt_constcharPtrWrap(flags),
+   libvirt_intWrap(iface-flags));
+
+PyDict_SetItem(py_retval, libvirt_constcharPtrWrap(iface-name),
+   pyIface);
+}
+
+full_free = false;
+
+
+cleanup:
+for (i = 0; i  i_retval; i++) {
+/* We don't want to free values we've just
+ * shared with python variables unless
+ * there was an error and hence we are
+ * returning PY_NONE or equivalent */
+if (full_free) {
+VIR_FREE(ifaces[i]-name);
+VIR_FREE(ifaces[i]-hwaddr);
+for (j = 0; j  ifaces[i]-ip_addrs_count; j++) {
+VIR_FREE(ifaces[i]-ip_addrs[j].addr);
+VIR_FREE(ifaces[i]-ip_addrs[j].dstaddr);
+}
+}
+

[libvirt] [PATCH v2 0/8] Dump domain's IP addresses

2013-01-03 Thread Michal Privoznik
It's been a while since I tried get this in.  I've reworked the
patches, the exposed API, and start new round of reviews.

Moreover, during work I've come to point, where extending qemu
guest agent seemed wise:

http://lists.nongnu.org/archive/html/qemu-devel/2012-12/msg03264.html

Therefore I am introducing 'flags' field, which can contain some
boolean values we don't have yet, e.g. IFF_UP, IFF_PROMISC, etc.

Re: 'dstaddr' field in struct _virDomainInterfaceIPAddress;
It's basically a join of:
union {
char *dstaddr; /* for IFF_POINTOPOINT interface */
char *broadaddr; /* for IFF_BROADCAST interface */
}

Since an interface cannot has both flags set (see man 3
getifaddrs) I've joined both into one 'char *dstaddr'. I know it
is not mnemonic as the union, so I left it for discussion. I can
change it if you want to.

diff to v1:
-don't return array of objects, but array of pointer to objects instead

Michal Privoznik (8):
  Introduce virDomainInterfacesAddresses API
  Introduce virDomainInterfaceFree API
  qemu_agent: Implement 'guest-network-get-interfaces' command handling
  qemu: Implement qemuDomainInterfacesAddresses
  virsh: Expose virDomainInterfacesAddresses
  remote: Implement virDomainInterfacesAddresses
  python: Expose virDomainInterfacesAddresses
  python: create example for dumping domain IP addresses

 daemon/remote.c | 140 
 examples/python/Makefile.am |   2 +-
 examples/python/README  |   1 +
 examples/python/domipaddrs.py   |  50 
 include/libvirt/libvirt.h.in|  51 
 python/generator.py |   3 +
 python/libvirt-override-api.xml |   7 ++
 python/libvirt-override.c   | 120 
 src/driver.h|   6 ++
 src/libvirt.c   | 107 +
 src/libvirt_public.syms |   7 ++
 src/qemu/qemu_agent.c   | 171 
 src/qemu/qemu_agent.h   |   2 +
 src/qemu/qemu_driver.c  |  68 
 src/remote/remote_driver.c  |  94 ++
 src/remote/remote_protocol.x|  27 ++-
 src/remote_protocol-structs |  27 +++
 tools/virsh-domain.c| 104 
 18 files changed, 985 insertions(+), 2 deletions(-)
 create mode 100755 examples/python/domipaddrs.py

-- 
1.8.0.2

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


[libvirt] [PATCH v2 1/8] Introduce virDomainInterfacesAddresses API

2013-01-03 Thread Michal Privoznik
This API returns dynamically allocated array of IP addresses for
all domain interfaces.
---
 include/libvirt/libvirt.h.in | 49 ++
 python/generator.py  |  2 ++
 src/driver.h |  6 
 src/libvirt.c| 71 
 src/libvirt_public.syms  |  6 
 5 files changed, 134 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index cc3fe13..3ee6688 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4563,6 +4563,55 @@ int virDomainFSTrim(virDomainPtr dom,
 unsigned long long minimum,
 unsigned int flags);
 
+typedef enum {
+VIR_INTERFACE_BROADCAST = (1  0), /* Broadcast address valid. */
+VIR_INTERFACE_PPP = (1  1), /* Interface is point-to-point. */
+/* we can add new flags here */
+} virDomainInterfaceType;
+
+typedef enum {
+VIR_IP_ADDR_TYPE_IPV4,
+VIR_IP_ADDR_TYPE_IPV6,
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_IP_ADDR_TYPE_LAST
+#endif
+} virIPAddrType;
+
+
+typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
+typedef virDomainIPAddress *virDomainIPAddressPtr;
+struct _virDomainInterfaceIPAddress {
+int type;   /* virIPAddrType */
+char *addr; /* IP address */
+int prefix; /* IP address prefix */
+char *dstaddr;  /* Broadcast address (if @flags  VIR_INTERFACE_BROADCAST),
+   PPP destination address (if @flags  VIR_INTERFACE_PPP) 
*/
+};
+
+typedef struct _virDomainInterface virDomainInterface;
+typedef virDomainInterface *virDomainInterfacePtr;
+struct _virDomainInterface {
+char *name; /* interface name */
+unsigned int flags; /* virDomainInterfaceType */
+char *hwaddr;   /* hardware address */
+unsigned int ip_addrs_count;/* number of items in @ip_addr */
+virDomainIPAddressPtr ip_addrs; /* array of IP addresses */
+};
+
+typedef enum {
+VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT  = 0, /* hypervisor choice */
+VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT  = 1, /* use guest agent */
+VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER = 2, /* use nwfilter learning code 
*/
+/* etc ... */
+} virDomainInterfacesAddressesMethod;
+
+
+int virDomainInterfacesAddresses(virDomainPtr domain,
+ virDomainInterfacePtr **ifaces,
+ unsigned int method,
+ unsigned int flags);
+
 /**
  * virSchedParameterType:
  *
diff --git a/python/generator.py b/python/generator.py
index bae4edc..ee39e51 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -243,6 +243,7 @@ skipped_types = {
  'virEventHandleCallback': No function types in python,
  'virEventTimeoutCallback': No function types in python,
  'virDomainBlockJobInfoPtr': Not implemented yet,
+ 'virDomainInterfacePtr': Not implemented yet,
 }
 
 ###
@@ -428,6 +429,7 @@ skip_impl = (
 'virNodeGetMemoryParameters',
 'virNodeSetMemoryParameters',
 'virNodeGetCPUMap',
+'virDomainInterfacesAddresses',
 )
 
 qemu_skip_impl = (
diff --git a/src/driver.h b/src/driver.h
index 64d652f..dc93ffb 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -914,6 +914,11 @@ typedef int
   const char *mountPoint,
   unsigned long long minimum,
   unsigned int flags);
+typedef int
+(*virDrvDomainInterfacesAddresses)(virDomainPtr domain,
+   virDomainInterfacePtr **ifaces,
+   unsigned int method,
+   unsigned int flags);
 
 /**
  * _virDriver:
@@ -1107,6 +1112,7 @@ struct _virDriver {
 virDrvNodeGetCPUMap nodeGetCPUMap;
 virDrvDomainFSTrim  domainFSTrim;
 virDrvDomainSendProcessSignal   domainSendProcessSignal;
+virDrvDomainInterfacesAddresses domainInterfacesAddresses;
 };
 
 typedef int
diff --git a/src/libvirt.c b/src/libvirt.c
index bf674d1..6f0de36 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20424,3 +20424,74 @@ error:
 virDispatchError(dom-conn);
 return -1;
 }
+
+/**
+ * virDomainInterfacesAddresses:
+ * @domain: domain object
+ * @ifaces: array of @domain interfaces
+ * @method: which method use, one of virDomainInterfacesAddressesMethod
+ * @flags: extra flags, not used yet, so callers should always pass 0
+ *
+ * Return an array of interfaces presented in given @domain among with
+ * their IP and HW addresses. Single interface can have multiple or even
+ * none IP address.
+ *
+ * This API dynamically allocates the virDomainInterfacePtr struct based on
+ * how much interfaces domain has, usually there's 1:1 correlation between
+ * interfaces seen from the host and interfaces seen from the 

[libvirt] [PATCH v2 2/8] Introduce virDomainInterfaceFree API

2013-01-03 Thread Michal Privoznik
This is just a free function for array of virDomainInterfacePtr as
returned by virDomainInterfacesAddresses API.
---
 include/libvirt/libvirt.h.in |  2 ++
 python/generator.py  |  1 +
 src/libvirt.c| 36 
 src/libvirt_public.syms  |  1 +
 4 files changed, 40 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 3ee6688..4199cd3 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4612,6 +4612,8 @@ int virDomainInterfacesAddresses(virDomainPtr domain,
  unsigned int method,
  unsigned int flags);
 
+void virDomainInterfaceFree(virDomainInterfacePtr iface);
+
 /**
  * virSchedParameterType:
  *
diff --git a/python/generator.py b/python/generator.py
index ee39e51..e0a6f10 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -430,6 +430,7 @@ skip_impl = (
 'virNodeSetMemoryParameters',
 'virNodeGetCPUMap',
 'virDomainInterfacesAddresses',
+'virDomainInterfaceFree',
 )
 
 qemu_skip_impl = (
diff --git a/src/libvirt.c b/src/libvirt.c
index 6f0de36..6f4ce60 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20495,3 +20495,39 @@ error:
 virDispatchError(domain-conn);
 return -1;
 }
+
+/**
+ * virDomainInterfaceFree:
+ * @iface: array of interfaces
+ *
+ * Free an interface as returned by virDomainInterfacesAddresses.
+ * The @ifaces pointer is freed and should not be used thereafter.
+ * Basic usage is:
+ *
+ * virDomainInterfacePtr *iface = NULL;
+ * int i, j;
+ *
+ * i = virDomainInterfacesAddresses(dom, iface, method, flags);
+ *
+ * Do something here ...;
+ *
+ * for (j = 0; j  i; j ++)
+ *   virDomainInterfaceFree(iface[j]);
+ * free(iface);
+ */
+void
+virDomainInterfaceFree(virDomainInterfacePtr iface)
+{
+int i;
+VIR_DEBUG(iface=%p, iface);
+
+
+VIR_FREE(iface-name);
+VIR_FREE(iface-hwaddr);
+for (i = 0; i  iface-ip_addrs_count; i++) {
+VIR_FREE(iface-ip_addrs[i].addr);
+VIR_FREE(iface-ip_addrs[i].dstaddr);
+}
+VIR_FREE(iface-ip_addrs);
+VIR_FREE(iface);
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 8e7c5d2..a401363 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -583,6 +583,7 @@ LIBVIRT_1.0.1 {
 LIBVIRT_1.0.2 {
 global:
 virDomainInterfacesAddresses;
+virDomainInterfaceFree;
 } LIBVIRT_1.0.1;
 
 
-- 
1.8.0.2

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


Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu

2013-01-03 Thread Ján Tomko
On 12/26/12 02:00, liguang wrote:
 @@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
   * When QEMU grows support for  1 PCI domain, then pci.0 change
   * to pciNN.0  where NN is the domain number
   */
 -if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS))
 +if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) {
 +virBufferAsprintf(buf, ,bus=pci-bridge%d, info-addr.pci.bus);
 +} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) {
  virBufferAsprintf(buf, ,bus=pci.0);

Is there any way (or plan) to use more pci buses with QEMU other than
with the pci bridges? If not, we could just name the bridges pci.%d. (If
we index the bridges from 1).

 -else
 +} else {
  virBufferAsprintf(buf, ,bus=pci);
 +}
  if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
  virBufferAddLit(buf, ,multifunction=on);
  else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF)
 @@ -3455,6 +3460,32 @@ error:
  return NULL;
  }


  
 +char *
 +qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev,
 + qemuCapsPtr caps, int idx)
 +{
 +virBuffer buf = VIR_BUFFER_INITIALIZER;
 +
 +virBufferAsprintf(buf, pci-bridge,chassis_nr=1);

The chassis number has to be unique for each bridge.

 +
 +if ((dev-type != VIR_DOMAIN_PCIBRIDGE_TYPE_ROOT) 
 +(qemuBuildDeviceAddressStr(buf, dev-info, caps)  0))
 +goto error;
 +else
 +virBufferAsprintf(buf, ,id=pci-bridge%d , idx);
 +
 +if (virBufferError(buf)) {
 +virReportOOMError();
 +goto error;
 +}
 +
 +return virBufferContentAndReset(buf);
 +
 +error:
 +virBufferFreeAndReset(buf);
 +return NULL;
 +
 +}

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


Re: [libvirt] [libvirt-php][PATCH 00/20] compilation under OS X and various

2013-01-03 Thread Michal Novotny
Hi Tugdual!

Thanks for the patch series. I've applied and tested it and it's working
great!

Thanks again for the series. I've noted your major contribution in the
AUTHORS file
with big thanks going to you :-)

Michal

On 01/03/2013 10:34 AM, Tugdual Saunier wrote:
 These patches are the result of my work to get libvirt-php compiled
 under OS X.
 As I suceeded to compiled it, I found some bugs/warnings
 that are also fixed in the following commits.
 The most important is probably the vnc_get_bitmap and vnc_refresh_screen now 
 working.

 Tugdual Saunier (20):
   Fixed compilation under OS X
   fixed a malloc_error_break in doc generation
   fixed wrong xpath in domain type retrieving
   fixed PHP warnings in uuid generation
   fixed warnings
   fixed resource counter changes in libvirt_list_active_domains
   init array only if needed in libvirt_list_inactive_domains
   fixed wrong maxsize type in libvirt_logfile_set
   fixed wrong types in libvirt_check_version
   fixed wrong byte_order determination under other OS by improving it
   fixed wrong test on virConnectClose return value
   fixed vnc_get_bitmap
   Fixed infinite loop in vnc_refresh_screen
   fixed last warnings
   fixed doc generation under OSX
   Made tests more universal by using libvirt test driver
   reorder tests by dependency
   improved test-install.phpt, no more need of a random name
   fixed some segfault in test-domain-snapshot.phpt
   fixed wrong doc alignment for libvirt_domain_managedsave

  configure.ac |  10 +
  docs/Makefile.am |  14 +-
  src/Makefile.am  |   5 +-
  src/libvirt-php.c| 330 
 +--
  src/libvirt-php.h|  16 +-
  src/vncfunc.c|  25 +-
  tests/data/example-no-disk-and-media.xml |   3 +-
  tests/functions.phpt |   8 +
  tests/php.ini|   2 +
  tests/runtests.sh|  11 +-
  tests/test-conn-limit.phpt   |   2 +-
  tests/test-connect.phpt  |  40 +---
  tests/test-domain-create-and-coredump.phpt   |   2 +-
  tests/test-domain-create-and-get-xpath.phpt  |   4 +-
  tests/test-domain-create.phpt|   2 +-
  tests/test-domain-define-create-destroy.phpt |   2 +-
  tests/test-domain-define-undefine.phpt   |   2 +-
  tests/test-domain-snapshot.phpt  |  14 +-
  tests/test-get-emulator.phpt |  10 +-
  tests/test-install.phpt  |  14 +-
  tests/test-logging.phpt  |   6 +-
  tools/generate-api-docs.c|   1 +
  22 files changed, 260 insertions(+), 263 deletions(-)
  create mode 100644 tests/data/test-libvirt-php.img
  create mode 100644 tests/php.ini

 --
 1.8.0.1


-- 
Michal Novotny minov...@redhat.com, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

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


[libvirt] [PATCH 0/2] Split out parts of qemu_driver.c into separate files

2013-01-03 Thread Peter Krempa
qemu_driver.c is starting to grow out of proportions. This series
creates two(actualy four) files to split out some code from the main file.

1) qemu_util.c - various qemu-related tools and common code (~0.5k lines)
2) qemu_snapshot.c - snapshot related code (~1.7k lines)

Apart from reformating function headers, no changes were made to the code.

Peter Krempa (2):
  qemu: Split out various utility functions to qemu_util.c
  qemu: Split out snapshot related functions to qemu_snapshot.c

 po/POTFILES.in   |2 +
 src/Makefile.am  |2 +
 src/qemu/qemu_driver.c   | 2191 +-
 src/qemu/qemu_snapshot.c | 1752 
 src/qemu/qemu_snapshot.h |   38 +
 src/qemu/qemu_util.c |  486 ++
 src/qemu/qemu_util.h |  111 +++
 7 files changed, 2404 insertions(+), 2178 deletions(-)
 create mode 100644 src/qemu/qemu_snapshot.c
 create mode 100644 src/qemu/qemu_snapshot.h
 create mode 100644 src/qemu/qemu_util.c
 create mode 100644 src/qemu/qemu_util.h

-- 
1.8.0.2
e

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


[libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c

2013-01-03 Thread Peter Krempa
---
 po/POTFILES.in |   1 +
 src/Makefile.am|   1 +
 src/qemu/qemu_driver.c | 492 ++---
 src/qemu/qemu_util.c   | 486 
 src/qemu/qemu_util.h   | 111 +++
 5 files changed, 611 insertions(+), 480 deletions(-)
 create mode 100644 src/qemu/qemu_util.c
 create mode 100644 src/qemu/qemu_util.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 4d94799..4edacfa 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -101,6 +101,7 @@ src/qemu/qemu_monitor.c
 src/qemu/qemu_monitor_json.c
 src/qemu/qemu_monitor_text.c
 src/qemu/qemu_process.c
+src/qemu/qemu_util.c
 src/remote/remote_client_bodies.h
 src/remote/remote_driver.c
 src/rpc/virkeepalive.c
diff --git a/src/Makefile.am b/src/Makefile.am
index f7a9b91..f76a2ea 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -515,6 +515,7 @@ QEMU_DRIVER_SOURCES =   
\
qemu/qemu_conf.c qemu/qemu_conf.h   \
qemu/qemu_process.c qemu/qemu_process.h \
qemu/qemu_migration.c qemu/qemu_migration.h \
+   qemu/qemu_util.c qemu/qemu_util.h   \
qemu/qemu_monitor.c qemu/qemu_monitor.h \
qemu/qemu_monitor_text.c\
qemu/qemu_monitor_text.h\
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a52b47..a9c03b6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1,7 +1,7 @@
 /*
  * qemu_driver.c: core driver methods for managing qemu guests
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -57,6 +57,7 @@
 #include qemu_bridge_filter.h
 #include qemu_process.h
 #include qemu_migration.h
+#include qemu_util.h

 #include virerror.h
 #include virlog.h
@@ -186,97 +187,6 @@ struct qemuAutostartData {
 virConnectPtr conn;
 };

-/**
- * qemuDomObjFromDomainDriver:
- * @domain: Domain pointer that has to be looked up
- * @drv: Pointer to virQEMUDriverPtr to return the driver object
- *
- * This function looks up @domain in the domain list and returns the
- * appropriate virDomainObjPtr. On successful lookup, both driver and domain
- * object are returned locked.
- *
- * Returns the domain object if it's found and the driver. Both are locked.
- * In case of failure NULL is returned and the driver isn't locked.
- */
-static virDomainObjPtr
-qemuDomObjFromDomainDriver(virDomainPtr domain, virQEMUDriverPtr *drv)
-{
-virQEMUDriverPtr driver = domain-conn-privateData;
-virDomainObjPtr vm;
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-
-qemuDriverLock(driver);
-vm = virDomainFindByUUID(driver-domains, domain-uuid);
-if (!vm) {
-virUUIDFormat(domain-uuid, uuidstr);
-virReportError(VIR_ERR_NO_DOMAIN,
-   _(no domain with matching uuid '%s'), uuidstr);
-qemuDriverUnlock(driver);
-*drv = NULL;
-return NULL;
-}
-
-*drv = driver;
-return vm;
-}
-
-/**
- * qemuDomObjFromDomain:
- * @domain: Domain pointer that has to be looked up
- *
- * This function looks up @domain and returns the appropriate
- * virDomainObjPtr. The driver is unlocked after the call.
- *
- * Returns the domain object which is locked on success, NULL
- * otherwise. The driver remains unlocked after the call.
- */
-static virDomainObjPtr
-qemuDomObjFromDomain(virDomainPtr domain)
-{
-virDomainObjPtr vm;
-virQEMUDriverPtr driver;
-
-if (!(vm = qemuDomObjFromDomainDriver(domain, driver)))
-return NULL;
-
-qemuDriverUnlock(driver);
-
-return vm;
-}
-
-/* Looks up the domain object from snapshot and unlocks the driver. The
- * returned domain object is locked and the caller is responsible for
- * unlocking it */
-static virDomainObjPtr
-qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot)
-{
-return qemuDomObjFromDomain(snapshot-domain);
-}
-
-
-/* Looks up snapshot object from VM and name */
-static virDomainSnapshotObjPtr
-qemuSnapObjFromName(virDomainObjPtr vm,
-const char *name)
-{
-virDomainSnapshotObjPtr snap = NULL;
-snap = virDomainSnapshotFindByName(vm-snapshots, name);
-if (!snap)
-virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
-   _(no domain snapshot with matching name '%s'),
-   name);
-
-return snap;
-}
-
-
-/* Looks up snapshot object from VM and snapshotPtr */
-static virDomainSnapshotObjPtr
-qemuSnapObjFromSnapshot(virDomainObjPtr vm,
-virDomainSnapshotPtr snapshot)
-{
-return qemuSnapObjFromName(vm, snapshot-name);
-}

 static void
 qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED,
@@ 

[libvirt] [PATCH] fwrite: silence __wur without using inline

2013-01-03 Thread Eric Blake
The libvirt folks reported[1] a link error of multiple rpl_fwrite
definitions that hits only when optimization and FORTIFY_SOURCE
are both enabled, due to improper use of inline.  But since that
particular use of rpl_fwrite exists only to work around a spurious
gcc warning on some versions of glibc, we can use gcc extensions
to acheive the same effect without using inline.  This approach
copies the method used in ignore-value.h.

[1] https://lists.gnu.org/archive/html/bug-gnulib/2013-01/msg00014.html

* lib/stdio.in.h (fwrite): Limit warn_unused_result workaround to
just gcc, and in a way that avoids inline issues.
* modules/stdio (Depends-on): Drop extern-inline.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 ChangeLog  |  7 +++
 lib/stdio.in.h | 23 +--
 modules/stdio  |  1 -
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 73c186d..0bae990 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2013-01-03  Eric Blake  ebl...@redhat.com
+
+   fwrite: silence __wur without using inline
+   * lib/stdio.in.h (fwrite): Limit warn_unused_result workaround to
+   just gcc, and in a way that avoids inline issues.
+   * modules/stdio (Depends-on): Drop extern-inline.
+
 2013-01-03  Jim Meyering  j...@meyering.net

update-copyright: avoid copyright notice date corruption
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index ec39c7d..0f20171 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -46,11 +46,6 @@
 #ifndef _@GUARD_PREFIX@_STDIO_H
 #define _@GUARD_PREFIX@_STDIO_H

-_GL_INLINE_HEADER_BEGIN
-#ifndef _GL_STDIO_INLINE
-# define _GL_STDIO_INLINE _GL_INLINE
-#endif
-
 /* Get va_list.  Needed on many systems, including glibc 2.8.  */
 #include stdarg.h

@@ -583,18 +578,12 @@ _GL_CXXALIAS_SYS (fwrite, size_t,
 /* Work around glibc bug 11959
http://sources.redhat.com/bugzilla/show_bug.cgi?id=11959,
which sometimes causes an unwanted diagnostic for fwrite calls.
-   This affects only function declaration attributes, so it's not
-   needed for C++.  */
-#  if !defined __cplusplus  0  __USE_FORTIFY_LEVEL
-_GL_STDIO_INLINE size_t _GL_ARG_NONNULL ((1, 4))
-rpl_fwrite (const void *ptr, size_t s, size_t n, FILE *stream)
-{
-  size_t r = fwrite (ptr, s, n, stream);
-  (void) r;
-  return r;
-}
+   This affects only function declaration attributes under certain
+   versions of gcc, and is not needed for C++.  */
+#  if !defined __cplusplus  0  __USE_FORTIFY_LEVEL \
+   3  (__GNUC__ + (4 = __GNUC_MINOR__))
 #   undef fwrite
-#   define fwrite rpl_fwrite
+#   define fwrite(a, b, c, d) ({size_t __r = fwrite (a, b, c, d); __r; })
 #  endif
 # endif
 _GL_CXXALIASWARN (fwrite);
@@ -1338,8 +1327,6 @@ _GL_WARN_ON_USE (vsprintf, vsprintf is not always POSIX 
compliant - 
   POSIX compliance);
 #endif

-_GL_INLINE_HEADER_END
-
 #endif /* _@GUARD_PREFIX@_STDIO_H */
 #endif /* _@GUARD_PREFIX@_STDIO_H */
 #endif
diff --git a/modules/stdio b/modules/stdio
index c33ad31..54189dc 100644
--- a/modules/stdio
+++ b/modules/stdio
@@ -7,7 +7,6 @@ lib/stdio.in.h
 m4/stdio_h.m4

 Depends-on:
-extern-inline
 include_next
 snippet/arg-nonnull
 snippet/c++defs
-- 
1.8.0.2

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


[libvirt] [PATCH] build: fix build with optimization enabled

2013-01-03 Thread Eric Blake
Build failure detected by Jenkins autobuilder:
http://honk.sigxcpu.org:8001/job/libvirt-build/472/changes

* .gnulib: Update to latest, for stdio.h rpl_fwrite fix.
---

Pushing under the build-breaker rule.  I managed to reproduce
the link failure on a RHEL 6 box with CFLAGS='-g -O2', and
also that this gnulib update made it go away.

* .gnulib 964bbc2...61c7b1e (2):
   fwrite: silence __wur without using inline
   update-copyright: avoid copyright notice date corruption

 .gnulib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gnulib b/.gnulib
index 964bbc2..61c7b1e 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 964bbc2d419584e93fe629ddbc40595612f62083
+Subproject commit 61c7b1e32e11e9e40b4d59ab888a807620befcd3
-- 
1.8.0.2

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


Re: [libvirt] libvirtd segfault

2013-01-03 Thread Scott Sullivan

On 01/02/2013 09:45 AM, Scott Sullivan wrote:

On 12/29/2012 04:09 AM, Michal Privoznik wrote:

On 28.12.2012 20:23, Scott Sullivan wrote:
snip/

I have just now received another SIGSEGV, with your patch applied.

Here's the info from the GDB session:

Detaching after fork from child process 11266.
2012-12-28 18:56:53.261+: 29943: error : qemuMonitorIO:614 :
internal error End of file from monitor

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffec0cd700 (LWP 29955)]
qemuDomainObjBeginJobInternal (driver=0x7fffe4013520,
driver_locked=true, obj=0x7fff7801fc80, job=QEMU_JOB_DESTROY,
asyncJob=QEMU_ASYNC_JOB_NONE) at qemu/qemu_domain.c:780
780 priv-jobs_queued++;
(gdb) bt
#0  qemuDomainObjBeginJobInternal (driver=0x7fffe4013520, 
driver_locked=true, obj=0x7fff7801fc80, job=QEMU_JOB_DESTROY, 
asyncJob=QEMU_ASYNC_JOB_NONE) at qemu/qemu_domain.c:780
#1  0x7fffea599f46 in qemuDomainDestroyFlags (dom=value 
optimized out, flags=value optimized out) at qemu/qemu_driver.c:2189
#2  0x77a83587 in virDomainDestroy (domain=0x7fffe414a510) 
at libvirt.c:2215
#3  0x004296e2 in remoteDispatchDomainDestroy (server=value 
optimized out, client=value optimized out, msg=value optimized 
out, rerr=0x7fffec0ccbc0, args=value optimized out, ret=value 
optimized out) at remote_dispatch.h:1277
#4  remoteDispatchDomainDestroyHelper (server=value optimized out, 
client=value optimized out, msg=value optimized out, 
rerr=0x7fffec0ccbc0, args=value optimized out, ret=value 
optimized out) at remote_dispatch.h:1255
#5  0x77ad0d02 in virNetServerProgramDispatchCall 
(prog=0x6814d0, server=0x678df0, client=0x693a80, msg=0x6986d0) at 
rpc/virnetserverprogram.c:431
#6  virNetServerProgramDispatch (prog=0x6814d0, server=0x678df0, 
client=0x693a80, msg=0x6986d0) at rpc/virnetserverprogram.c:304
#7  0x77aceaa6 in virNetServerProcessMsg (srv=value 
optimized out, client=0x693a80, prog=value optimized out, 
msg=0x6986d0) at rpc/virnetserver.c:173
#8  0x77acf5e3 in virNetServerHandleJob (jobOpaque=value 
optimized out, opaque=0x678df0) at rpc/virnetserver.c:194
#9  0x779e8fdc in virThreadPoolWorker (opaque=value 
optimized out) at util/threadpool.c:144
#10 0x779e88c9 in virThreadHelper (data=value optimized 
out) at util/threads-pthread.c:161

#11 0x00300a2077f1 in start_thread () from /lib64/libpthread.so.0
#12 0x003009ae570d in clone () from /lib64/libc.so.6
(gdb)

This means, even though we successfully incremented reference counter on
virDomainObjPtr object, somebody free()d it anyway (well, the
privateData at least). Looks like a locking/concurrent access issue to
me then. Unfortunately, I don't have any suggestions yet, as the domain
object is supposed to be locked when entering the
qemuDomainObjBeginJobInternal() function so it shouldn't get free()d
meanwhile.

Michal

Michal,

I have a faster way to reproduce the crash (~10 minutes). Continue to 
read for new (easier) steps.
This test was done with the standard v1.0.0 libvirtd code source, with 
no other patches applied.


snip



Not sure how much this helps, but in my testing I have found this issue 
was introduced with v0.9.12.


I cannot reproduce this issue under v0.9.11.X or older. Comparing 
src/qemu/qemu_domain.c between v0.9.11.X and v0.9.12 I see numerous 
changes to the code related to locking/concurrency.  For instance, the 
introduction of qemuDomainTrackJob as one large difference I see.



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


Re: [libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c

2013-01-03 Thread Eric Blake
On 01/03/2013 09:45 AM, Peter Krempa wrote:
 ---
  po/POTFILES.in |   1 +
  src/Makefile.am|   1 +
  src/qemu/qemu_driver.c | 492 
 ++---
  src/qemu/qemu_util.c   | 486 
  src/qemu/qemu_util.h   | 111 +++
  5 files changed, 611 insertions(+), 480 deletions(-)
  create mode 100644 src/qemu/qemu_util.c
  create mode 100644 src/qemu/qemu_util.h
 

 
 +
 +static inline void
 +bswap_header(virQEMUSaveHeaderPtr hdr) {

It looks weird that you moved this function, but only to later in the
same file rather than into a new file, and also weird that this function
doesn't follow naming conventions.  I would leave it untouched in this
patch, and then do a followup patch that inlines the body of this
function into its lone caller (qemuDomainSaveImageOpen); which will get
rid of the bad naming at the same time.

 +++ b/src/qemu/qemu_util.c
 @@ -0,0 +1,486 @@
 +/*
 + * qemu_util.c: Various util functions for the QEMU driver
 + *
 + * Copyright (C) 2013 Red Hat, Inc.

Although this file only exists in 2013, it copies from other files with
earlier copyrights.  Please preserve all of the copyrights from
qemu_driver.c (or, for bonus points, take the time to audit which year
each individual function was added and later modified while it lived
within qemu_driver.c).  Same to the qemu_util.h file.

 +
 +#include config.h
 +
 +#include sys/types.h
 +#include sys/stat.h
 +#include unistd.h
 +#include byteswap.h
 +#include fcntl.h
 +
 +#include qemu_util.h
 +#include qemu_cgroup.h
 +#include qemu_migration.h
 +
 +#include internal.h
 +
 +#include virerror.h
 +#include domain_conf.h
 +#include locking/domain_lock.h
 +#include datatypes.h
 +#include virlog.h
 +#include viralloc.h
 +#include virfile.h

Do we need all of these, or can it be trimmed a bit?  Should we sort things?

 +++ b/src/qemu/qemu_util.h
 @@ -0,0 +1,111 @@

 +typedef enum {
 +QEMU_SAVE_FORMAT_RAW = 0,
 +QEMU_SAVE_FORMAT_GZIP = 1,
 +QEMU_SAVE_FORMAT_BZIP2 = 2,
 +/*
 + * Deprecated by xz and never used as part of a release
 + * QEMU_SAVE_FORMAT_LZMA
 + */

This comment is safe to drop now.  We have version control for a reason.

ACK with those points addressed.

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



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

Re: [libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 05:45:49PM +0100, Peter Krempa wrote:
 ---
  po/POTFILES.in |   1 +
  src/Makefile.am|   1 +
  src/qemu/qemu_driver.c | 492 
 ++---
  src/qemu/qemu_util.c   | 486 
  src/qemu/qemu_util.h   | 111 +++
  5 files changed, 611 insertions(+), 480 deletions(-)
  create mode 100644 src/qemu/qemu_util.c
  create mode 100644 src/qemu/qemu_util.h

 +# include qemu_domain.h
 +
 +/* It would be nice to replace 'Qemud' with 'Qemu' but
 + * this magic string is ABI, so it can't be changed
 + */
 +# define QEMU_SAVE_MAGIC   LibvirtQemudSave
 +# define QEMU_SAVE_PARTIAL LibvirtQemudPart
 +# define QEMU_SAVE_VERSION 2
 +
 +verify(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL));
 +
 +typedef struct _virQEMUSaveHeader virQEMUSaveHeader;
 +typedef virQEMUSaveHeader *virQEMUSaveHeaderPtr;
 +struct _virQEMUSaveHeader {
 +char magic[sizeof(QEMU_SAVE_MAGIC)-1];
 +uint32_t version;
 +uint32_t xml_len;
 +uint32_t was_running;
 +uint32_t compressed;
 +uint32_t unused[15];
 +};
 +
 +
 +typedef enum {
 +VIR_DISK_CHAIN_NO_ACCESS,
 +VIR_DISK_CHAIN_READ_ONLY,
 +VIR_DISK_CHAIN_READ_WRITE,
 +} qemuDomainDiskChainMode;
 +
 +typedef enum {
 +QEMU_SAVE_FORMAT_RAW = 0,
 +QEMU_SAVE_FORMAT_GZIP = 1,
 +QEMU_SAVE_FORMAT_BZIP2 = 2,
 +/*
 + * Deprecated by xz and never used as part of a release
 + * QEMU_SAVE_FORMAT_LZMA
 + */
 +QEMU_SAVE_FORMAT_XZ = 3,
 +QEMU_SAVE_FORMAT_LZOP = 4,
 +/* Note: add new members only at the end.
 +   These values are used in the on-disk format.
 +   Do not change or re-use numbers. */
 +
 +QEMU_SAVE_FORMAT_LAST
 +} virQEMUSaveFormat;
 +
 +VIR_ENUM_DECL(qemuSaveCompression)
 +
 +
 +virDomainObjPtr qemuDomObjFromDomainDriver(virDomainPtr domain,
 +   virQEMUDriverPtr *drv);
 +
 +virDomainObjPtr qemuDomObjFromDomain(virDomainPtr domain);
 +
 +virDomainObjPtr qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot);
 +
 +virDomainSnapshotObjPtr qemuSnapObjFromSnapshot(virDomainObjPtr vm,
 +virDomainSnapshotPtr 
 snapshot);
 +
 +virDomainSnapshotObjPtr qemuSnapObjFromName(virDomainObjPtr vm,
 +const char *name);

All these should go in qemu_domain.{c,h}


 +int qemuOpenFile(virQEMUDriverPtr driver,
 + const char *path,
 + int oflags,
 + bool *needUnlink,
 + bool *bypassSecurityDriver);

qemu_conf.c

 +int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
 +  virDomainObjPtr vm,
 +  virCgroupPtr cgroup,
 +  virDomainDiskDefPtr disk,
 +  const char *file,
 +  qemuDomainDiskChainMode mode);

qemu_domain.h


 +int qemuDomainSaveMemory(virQEMUDriverPtr driver,
 + virDomainObjPtr vm,
 + const char *path,
 + const char *domXML,
 + int compressed,
 + bool was_running,
 + unsigned int flags,
 + enum qemuDomainAsyncJob asyncJob);

Not convinced this needs to move at all.

 +const char *qemuCompressProgramName(int compress);

qemu_domain.h



In summary, I don't think we should create a qemu_util.{c,h} file - any
file named util just ends up as a garbage dumping ground for code that
should be better placed elsewhere. See also util/util.h which should be
split up.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 2/2] qemu: Split out snapshot related functions to qemu_snapshot.c

2013-01-03 Thread Eric Blake
On 01/03/2013 09:45 AM, Peter Krempa wrote:
 ---
  po/POTFILES.in   |1 +
  src/Makefile.am  |1 +
  src/qemu/qemu_driver.c   | 1699 +---
  src/qemu/qemu_snapshot.c | 1752 
 ++
  src/qemu/qemu_snapshot.h |   38 +
  5 files changed, 1793 insertions(+), 1698 deletions(-)
  create mode 100644 src/qemu/qemu_snapshot.c
  create mode 100644 src/qemu/qemu_snapshot.h

Now that I've seen Dan's arguments against 1/2, I tend to agree that
qemu_util.c doesn't make much sense.  But qemu_snapshot.c still makes
sense, so I will still review this one.

 +++ b/src/qemu/qemu_snapshot.c
 @@ -0,0 +1,1752 @@
 +/*
 + * qemu_snapshot.c: QEMU snapshot handling
 + *
 + * Copyright (C) 2013 Red Hat, Inc.

Same comments as in 1/2 about copying over copyright years from the
original location.

 +
 +#include config.h
 +
 +#include sys/types.h
 +#include sys/stat.h
 +#include fcntl.h
 +#include unistd.h
 +

And same comment about sorting includes.

 +#ifndef __QEMU_SNAPSHOT_H__
 +# define __QEMU_SNAPSHOT_H__
 +
 +# include qemu_domain.h
 +
 +virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
 + const char *xmlDesc,
 + unsigned int flags);
 +
 +int qemuDomainRevertToSnapshot(virDomainSnapshotPtr,
 +   unsigned int flags);
 +
 +int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
 + unsigned int flags);
 +

Interesting subset.  Why not move any of these other snapshot-related
driver callbacks?  qemuDomainSnapshotGetXMLDesc, qemuDomainSnapshotNum,
qemuDomainSnapshotListNames, qemuDomainListAllSnapshots,
qemuDomainSnapshotNumChildren, qemuDomainSnapshotListChildrenNames,
qemuDomainSnapshotListAllChildren, qemuDomainSnapshotLookupByName,
qemuDomainSnapshotGetParent, qemuDomainSnapshotCurrent,
qemuDomainSnapshotIsCurrent, qemuDomainSnapshotHasMetadata

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



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

Re: [libvirt] [PATCH 2/2] qemu: Split out snapshot related functions to qemu_snapshot.c

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 10:51:09AM -0700, Eric Blake wrote:
 On 01/03/2013 09:45 AM, Peter Krempa wrote:
  ---
   po/POTFILES.in   |1 +
   src/Makefile.am  |1 +
   src/qemu/qemu_driver.c   | 1699 
  +---
   src/qemu/qemu_snapshot.c | 1752 
  ++
   src/qemu/qemu_snapshot.h |   38 +
   5 files changed, 1793 insertions(+), 1698 deletions(-)
   create mode 100644 src/qemu/qemu_snapshot.c
   create mode 100644 src/qemu/qemu_snapshot.h
 
 Now that I've seen Dan's arguments against 1/2, I tend to agree that
 qemu_util.c doesn't make much sense.  But qemu_snapshot.c still makes
 sense, so I will still review this one.
 
  +++ b/src/qemu/qemu_snapshot.c
  @@ -0,0 +1,1752 @@
  +/*
  + * qemu_snapshot.c: QEMU snapshot handling
  + *
  + * Copyright (C) 2013 Red Hat, Inc.
 
 Same comments as in 1/2 about copying over copyright years from the
 original location.
 
  +
  +#include config.h
  +
  +#include sys/types.h
  +#include sys/stat.h
  +#include fcntl.h
  +#include unistd.h
  +
 
 And same comment about sorting includes.
 
  +#ifndef __QEMU_SNAPSHOT_H__
  +# define __QEMU_SNAPSHOT_H__
  +
  +# include qemu_domain.h
  +
  +virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
  + const char *xmlDesc,
  + unsigned int flags);
  +
  +int qemuDomainRevertToSnapshot(virDomainSnapshotPtr,
  +   unsigned int flags);
  +
  +int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
  + unsigned int flags);
  +
 
 Interesting subset.  Why not move any of these other snapshot-related
 driver callbacks?  qemuDomainSnapshotGetXMLDesc, qemuDomainSnapshotNum,
 qemuDomainSnapshotListNames, qemuDomainListAllSnapshots,
 qemuDomainSnapshotNumChildren, qemuDomainSnapshotListChildrenNames,
 qemuDomainSnapshotListAllChildren, qemuDomainSnapshotLookupByName,
 qemuDomainSnapshotGetParent, qemuDomainSnapshotCurrent,
 qemuDomainSnapshotIsCurrent, qemuDomainSnapshotHasMetadata

IMHO all methods which are directly part of the public API
driver struct should be in qemu_driver.c. If we want to split
code, then there should be a set of internal helpers which
those public API methods call.

eg, see how it is done for migration.

  qemu_driver.c contains qemuDomainMigratePrepare2

which in turn calls

  qemuMigrationPrepareDirect in qemu_migration.c

Basically all the qemu_driver.c code does is convert from the public
API types (virDomainPtr) into the private API types (virDomainObjPtr)
and then call the main impl code.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


[libvirt] [PATCH] qemu: fix a segfault in qemuProcessWaitForMonitor

2013-01-03 Thread Ján Tomko
Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in
the case of QMP capability probing being used, leading to a segfault in
strlen in the cleanup path.

This patch opens the log and allocates the buffer if QMP probing was
used, so we can display the helpful error message.
---
 src/qemu/qemu_process.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 358757b..2d63cf2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1709,6 +1709,15 @@ cleanup:
 if (pos != -1  kill(vm-pid, 0) == -1  errno == ESRCH) {
 /* VM is dead, any other error raised in the interim is probably
  * not as important as the qemu cmdline output */
+if (qemuCapsUsedQMP(caps)) {
+if ((logfd = qemuDomainOpenLog(driver, vm, pos))  0)
+return -1;
+
+if (VIR_ALLOC_N(buf, buf_size)  0) {
+virReportOOMError();
+goto closelog;
+}
+}
 qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf));
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(process exited while connecting to monitor: %s),
-- 
1.7.8.6

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


Re: [libvirt] [PATCH] fwrite: silence __wur without using inline

2013-01-03 Thread Paul Eggert
Thanks for fixing that.  I'm still puzzled about
why the problem happened with libvirt.  It's better now that
stdio doesn't depend on extern-inline, but I worry that whatever
bug the libvirt folks were having with stdio and extern inline
might crop up when extern inline is used in other include files.

One minor improvement is that we can limit the workaround to
glibc versions that have the problem, so I pushed this further
change.

---
 ChangeLog  | 6 ++
 lib/stdio.in.h | 8 +---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0bae990..714ee4c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2013-01-03  Paul Eggert  egg...@cs.ucla.edu
+
+   fwrite: silence __wur only for older glibc versions
+   * lib/stdio.in.h (fwrite): Limit workaround to glibc 2.4 through 2.15.
+   This will help us remove this workaround some time in the far future.
+
 2013-01-03  Eric Blake  ebl...@redhat.com
 
fwrite: silence __wur without using inline
diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index f9765d1..c345499 100644
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -575,13 +575,15 @@ _GL_CXXALIAS_RPL (fwrite, size_t,
 _GL_CXXALIAS_SYS (fwrite, size_t,
   (const void *ptr, size_t s, size_t n, FILE *stream));
 
-/* Work around glibc bug 11959
+/* Work around bug 11959 when fortifying glibc 2.4 through 2.15
http://sources.redhat.com/bugzilla/show_bug.cgi?id=11959,
which sometimes causes an unwanted diagnostic for fwrite calls.
This affects only function declaration attributes under certain
versions of gcc, and is not needed for C++.  */
-#  if !defined __cplusplus  0  __USE_FORTIFY_LEVEL \
-   3  (__GNUC__ + (4 = __GNUC_MINOR__))
+#  if (0  __USE_FORTIFY_LEVEL  \
+__GLIBC__ == 2  4 = __GLIBC_MINOR__  __GLIBC_MINOR__ = 15 \
+3  __GNUC__ + (4 = __GNUC_MINOR__)  \
+!defined __cplusplus)
 #   undef fwrite
 #   define fwrite(a, b, c, d) ({size_t __r = fwrite (a, b, c, d); __r; })
 #  endif
-- 
1.7.11.7


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


Re: [libvirt] [PATCH 1/2] qemu: Split out various utility functions to qemu_util.c

2013-01-03 Thread Peter Krempa

On 01/03/13 18:37, Daniel P. Berrange wrote:

On Thu, Jan 03, 2013 at 05:45:49PM +0100, Peter Krempa wrote:

---
  po/POTFILES.in |   1 +
  src/Makefile.am|   1 +
  src/qemu/qemu_driver.c | 492 ++---
  src/qemu/qemu_util.c   | 486 
  src/qemu/qemu_util.h   | 111 +++
  5 files changed, 611 insertions(+), 480 deletions(-)
  create mode 100644 src/qemu/qemu_util.c
  create mode 100644 src/qemu/qemu_util.h




...



All these should go in qemu_domain.{c,h}



Fair enough.




+int qemuOpenFile(virQEMUDriverPtr driver,
+ const char *path,
+ int oflags,
+ bool *needUnlink,
+ bool *bypassSecurityDriver);


qemu_conf.c


Hm, qemu_conf.c, okay it's used to open files honoring the config ...




+int qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virCgroupPtr cgroup,
+  virDomainDiskDefPtr disk,
+  const char *file,
+  qemuDomainDiskChainMode mode);


qemu_domain.h



+int qemuDomainSaveMemory(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *path,
+ const char *domXML,
+ int compressed,
+ bool was_running,
+ unsigned int flags,
+ enum qemuDomainAsyncJob asyncJob);


Not convinced this needs to move at all.


This one needs to be included into qemu_snapshot.c that is created in 
2/2 of this series. All functions split out in this patch were chosen to 
allow that.





+const char *qemuCompressProgramName(int compress);


qemu_domain.h


In summary, I don't think we should create a qemu_util.{c,h} file - any
file named util just ends up as a garbage dumping ground for code that
should be better placed elsewhere. See also util/util.h which should be
split up.


Well, as you can see it will ultimately end up somewhere. Unfortunately 
it this case everything tends to end up in qemu_driver.c.




Daniel



Peter

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


[libvirt] Build failed in Jenkins: libvirt-check #441

2013-01-03 Thread Jenkins CI
See http://honk.sigxcpu.org:8001/job/libvirt-check/441/

--
Started by upstream project libvirt-build build number 482
[workspace] $ /bin/sh -xe /tmp/hudson1110434311877203261.sh
+ make check
Making check in gnulib/lib
make[1]: Entering directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib'
make  check-am
make[2]: Entering directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib'
make[2]: Nothing to be done for `check-am'.
make[2]: Leaving directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib'
make[1]: Leaving directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/gnulib/lib'
Making check in include
make[1]: Entering directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include'
Making check in libvirt
make[2]: Entering directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include/libvirt'
make[2]: Nothing to be done for `check'.
make[2]: Leaving directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include/libvirt'
make[2]: Entering directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include'
make[2]: Nothing to be done for `check-am'.
make[2]: Leaving directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include'
make[1]: Leaving directory 
`http://honk.sigxcpu.org:8001/job/libvirt-check/ws/include'
Making check in src
/bin/bash: line 9: 28650 Segmentation fault  make $local_target
make: *** [check-recursive] Error 1
Build step 'Execute shell' marked build as failure

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


Re: [libvirt] [PATCH] build: fix build with optimization enabled

2013-01-03 Thread Guido Günther
On Thu, Jan 03, 2013 at 10:15:06AM -0700, Eric Blake wrote:
 Build failure detected by Jenkins autobuilder:
 http://honk.sigxcpu.org:8001/job/libvirt-build/472/changes
 
 * .gnulib: Update to latest, for stdio.h rpl_fwrite fix.

That fixed it. Thanks a lot! As of the latest update I'm seeing spurious
SIGSEGV on make invocations. I'll diagnose these first until I turn the
list mails back on.
Cheers,
 -- Guido

 ---
 
 Pushing under the build-breaker rule.  I managed to reproduce
 the link failure on a RHEL 6 box with CFLAGS='-g -O2', and
 also that this gnulib update made it go away.
 
 * .gnulib 964bbc2...61c7b1e (2):
fwrite: silence __wur without using inline
update-copyright: avoid copyright notice date corruption
 
  .gnulib | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/.gnulib b/.gnulib
 index 964bbc2..61c7b1e 16
 --- a/.gnulib
 +++ b/.gnulib
 @@ -1 +1 @@
 -Subproject commit 964bbc2d419584e93fe629ddbc40595612f62083
 +Subproject commit 61c7b1e32e11e9e40b4d59ab888a807620befcd3
 -- 
 1.8.0.2
 

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


[libvirt] Jenkins build is back to normal : libvirt-syntax-check #437

2013-01-03 Thread Jenkins CI
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/437/

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


Re: [libvirt] [PATCH v2 1/8] Introduce virDomainInterfacesAddresses API

2013-01-03 Thread Laine Stump
On 01/03/2013 08:46 AM, Michal Privoznik wrote:
 This API returns dynamically allocated array of IP addresses for
 all domain interfaces.
 ---
  include/libvirt/libvirt.h.in | 49 ++
  python/generator.py  |  2 ++
  src/driver.h |  6 
  src/libvirt.c| 71 
 
  src/libvirt_public.syms  |  6 
  5 files changed, 134 insertions(+)

 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index cc3fe13..3ee6688 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -4563,6 +4563,55 @@ int virDomainFSTrim(virDomainPtr dom,
  unsigned long long minimum,
  unsigned int flags);
  
 +typedef enum {
 +VIR_INTERFACE_BROADCAST = (1  0), /* Broadcast address valid. */
 +VIR_INTERFACE_PPP = (1  1), /* Interface is point-to-point. */

I think you mean VIR_INTERFACE_PTP, not _PPP.


 +/* we can add new flags here */
 +} virDomainInterfaceType;
 +
 +typedef enum {
 +VIR_IP_ADDR_TYPE_IPV4,
 +VIR_IP_ADDR_TYPE_IPV6,
 +
 +#ifdef VIR_ENUM_SENTINELS
 +VIR_IP_ADDR_TYPE_LAST
 +#endif
 +} virIPAddrType;
 +
 +
 +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
 +typedef virDomainIPAddress *virDomainIPAddressPtr;
 +struct _virDomainInterfaceIPAddress {
 +int type;   /* virIPAddrType */
 +char *addr; /* IP address */
 +int prefix; /* IP address prefix */
 +char *dstaddr;  /* Broadcast address (if @flags  
 VIR_INTERFACE_BROADCAST),
 +   PPP destination address (if @flags  
 VIR_INTERFACE_PPP) */
 +};

I dislike creating yet another struct to represent IP address info when
we already can do that internally with virSocketAddr, but virSocketAddr
is too complicated (and has platform-specific bits to boot) to put it in
the public API, and every other IP address in the public API is
represented with XML, so I guess this is okay.

 +
 +typedef struct _virDomainInterface virDomainInterface;
 +typedef virDomainInterface *virDomainInterfacePtr;
 +struct _virDomainInterface {
 +char *name; /* interface name */
 +unsigned int flags; /* virDomainInterfaceType */
 +char *hwaddr;   /* hardware address */
 +unsigned int ip_addrs_count;/* number of items in @ip_addr */
 +virDomainIPAddressPtr ip_addrs; /* array of IP addresses */
 +};
 +
 +typedef enum {
 +VIR_DOMAIN_INTERFACE_ADDRS_DEFAULT  = 0, /* hypervisor choice */
 +VIR_DOMAIN_INTERFACE_ADDRS_GUEST_AGENT  = 1, /* use guest agent */
 +VIR_DOMAIN_INTERFACE_ADDRS_NWFILTER = 2, /* use nwfilter learning 
 code */
 +/* etc ... */

You should remove the etc line.

 +} virDomainInterfacesAddressesMethod;
 +
 +
 +int virDomainInterfacesAddresses(virDomainPtr domain,
 + virDomainInterfacePtr **ifaces,
 + unsigned int method,
 + unsigned int flags);
 +
  /**
   * virSchedParameterType:
   *
 diff --git a/python/generator.py b/python/generator.py
 index bae4edc..ee39e51 100755
 --- a/python/generator.py
 +++ b/python/generator.py
 @@ -243,6 +243,7 @@ skipped_types = {
   'virEventHandleCallback': No function types in python,
   'virEventTimeoutCallback': No function types in python,
   'virDomainBlockJobInfoPtr': Not implemented yet,
 + 'virDomainInterfacePtr': Not implemented yet,
  }
  
  ###
 @@ -428,6 +429,7 @@ skip_impl = (
  'virNodeGetMemoryParameters',
  'virNodeSetMemoryParameters',
  'virNodeGetCPUMap',
 +'virDomainInterfacesAddresses',
  )
  
  qemu_skip_impl = (
 diff --git a/src/driver.h b/src/driver.h
 index 64d652f..dc93ffb 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -914,6 +914,11 @@ typedef int
const char *mountPoint,
unsigned long long minimum,
unsigned int flags);
 +typedef int
 +(*virDrvDomainInterfacesAddresses)(virDomainPtr domain,
 +   virDomainInterfacePtr **ifaces,
 +   unsigned int method,
 +   unsigned int flags);
  
  /**
   * _virDriver:
 @@ -1107,6 +1112,7 @@ struct _virDriver {
  virDrvNodeGetCPUMap nodeGetCPUMap;
  virDrvDomainFSTrim  domainFSTrim;
  virDrvDomainSendProcessSignal   domainSendProcessSignal;
 +virDrvDomainInterfacesAddresses domainInterfacesAddresses;
  };
  
  typedef int
 diff --git a/src/libvirt.c b/src/libvirt.c
 index bf674d1..6f0de36 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -20424,3 +20424,74 @@ error:
  virDispatchError(dom-conn);
  return -1;
  }
 +
 +/**
 + * virDomainInterfacesAddresses:


The repeated 

[libvirt] [PATCH 02/10] Check and handle error for virAsprintf() calls. Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated.

2013-01-03 Thread John Ferlan
---
 src/parallels/parallels_storage.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/parallels/parallels_storage.c 
b/src/parallels/parallels_storage.c
index e768d88..2908bee 100644
--- a/src/parallels/parallels_storage.c
+++ b/src/parallels/parallels_storage.c
@@ -146,7 +146,7 @@ static char *parallelsMakePoolName(virConnectPtr conn, 
const char *path)
 if (i == 0)
 name = strdup(path);
 else
-virAsprintf(name, %s-%u, path, i);
+ignore_value(virAsprintf(name, %s-%u, path, i));
 
 if (!name) {
 virReportOOMError();
@@ -310,8 +310,7 @@ static int parallelsAddDiskVolume(virStoragePoolObjPtr pool,
 if (VIR_ALLOC(def))
 goto no_memory;
 
-virAsprintf(def-name, %s-%s, dom-def-name, diskName);
-if (!def-name)
+if (virAsprintf(def-name, %s-%s, dom-def-name, diskName)  0)
 goto no_memory;
 
 def-type = VIR_STORAGE_VOL_FILE;
-- 
1.7.11.7

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


[libvirt] [PATCH 04/10] Ignore the return status check for vmwareUpdateVMStatus in convenience routine vmwareDomainObjListUpdateDomain

2013-01-03 Thread John Ferlan
---
 src/vmware/vmware_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 3763f40..9c81df8 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -949,7 +949,7 @@ static void vmwareDomainObjListUpdateDomain(void *payload, 
const void *name ATTR
 struct vmware_driver *driver = data;
 virDomainObjPtr vm = payload;
 virDomainObjLock(vm);
-vmwareUpdateVMStatus(driver, vm);
+ignore_value(vmwareUpdateVMStatus(driver, vm));
 virDomainObjUnlock(vm);
 }
 
-- 
1.7.11.7

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


[libvirt] [PATCH 06/10] Check return on mkdir for LOCKSPACE_DIR

2013-01-03 Thread John Ferlan
---
 tests/virlockspacetest.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/virlockspacetest.c b/tests/virlockspacetest.c
index 8673700..7678396 100644
--- a/tests/virlockspacetest.c
+++ b/tests/virlockspacetest.c
@@ -293,7 +293,8 @@ static int testLockSpaceResourceLockPath(const void *args 
ATTRIBUTE_UNUSED)
 
 lockspace = virLockSpaceNew(NULL);
 
-mkdir(LOCKSPACE_DIR, 0700);
+if (mkdir(LOCKSPACE_DIR, 0700)  0)
+goto cleanup;
 
 if (virLockSpaceCreateResource(lockspace, LOCKSPACE_DIR /foo)  0)
 goto cleanup;
-- 
1.7.11.7

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


[libvirt] [PATCH 07/10] Check and handle error for virAsprintf() calls.

2013-01-03 Thread John Ferlan
---
 tests/vmx2xmltest.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 03a8989..2430350 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -179,7 +179,9 @@ testParseVMXFileName(const char *fileName, void *opaque 
ATTRIBUTE_UNUSED)
 goto cleanup;
 }
 
-virAsprintf(src, [%s] %s, datastoreName, directoryAndFileName);
+if (virAsprintf(src, [%s] %s, datastoreName,
+directoryAndFileName)  0)
+goto cleanup;
 } else if (STRPREFIX(fileName, /)) {
 /* Found absolute path referencing a file outside a datastore */
 src = strdup(fileName);
@@ -188,7 +190,8 @@ testParseVMXFileName(const char *fileName, void *opaque 
ATTRIBUTE_UNUSED)
 src = NULL;
 } else {
 /* Found single file name referencing a file inside a datastore */
-virAsprintf(src, [datastore] directory/%s, fileName);
+if (virAsprintf(src, [datastore] directory/%s, fileName)  0)
+goto cleanup;
 }
 
   cleanup:
-- 
1.7.11.7

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


[libvirt] [PATCH 00/10] Resolve CHECKED_RETURN errors found by Coverity

2013-01-03 Thread John Ferlan
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=880388

This set of patches resolves the CHECKED_RETURN (CWE-252) errors found
by Coverity.


John Ferlan (10):
  interface: Check and handle error for virAsprintf() calls.
  parallels: Check and handle error for virAsprintf() calls. Ignore the return 
   inparallelsMakePoolName() since subsequent check validates name
   was allocated.
  rpc: Check status when attempting to set SO_REUSEADDR flag on outgoing
   connection. On failure, VIR_WARN(), but continue to connect.
  vmware: Ignore the return status check for vmwareUpdateVMStatus in
   convenience routine vmwareDomainObjListUpdateDomain
  xen: Check return status for setting TCP_NODELAY option and generate a
   VIR_DEBUG message on failure. Allow connection to continue.
  virlockspacetest: Check return on mkdir for LOCKSPACE_DIR
  vmx2xmltest: Check and handle error for virAsprintf() calls.
  xml2vmxtest: Check and handle error for virAsprintf() calls.
  virsh: Ignore error returns for virBufferTrim().
  phyp: Check and handle select() errors from waitsocket().

 src/interface/interface_backend_udev.c |  5 ++--
 src/parallels/parallels_storage.c  |  5 ++--
 src/phyp/phyp_driver.c | 42 ++
 src/rpc/virnetsocket.c |  4 +++-
 src/vmware/vmware_driver.c |  2 +-
 src/xen/xend_internal.c|  6 +++--
 tests/virlockspacetest.c   |  3 ++-
 tests/vmx2xmltest.c|  7 --
 tests/xml2vmxtest.c|  5 ++--
 tools/virsh.c  |  4 ++--
 10 files changed, 56 insertions(+), 27 deletions(-)

-- 
1.7.11.7

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


[libvirt] [PATCH 09/10] Ignore error returns for virBufferTrim().

2013-01-03 Thread John Ferlan
---
 tools/virsh.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 283194a..9f834e4 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -544,7 +544,7 @@ vshTreePrintInternal(vshControl *ctl,
  false, indent)  0)
 goto cleanup;
 }
-virBufferTrim(indent,   , -1);
+ignore_value(virBufferTrim(indent,   , -1));
 
 /* If there was no child device, and we're the last in
  * a list of devices, then print another blank line */
@@ -552,7 +552,7 @@ vshTreePrintInternal(vshControl *ctl,
 vshPrint(ctl, %s\n, virBufferCurrentContent(indent));
 
 if (!root)
-virBufferTrim(indent, NULL, 2);
+ignore_value(virBufferTrim(indent, NULL, 2));
 ret = 0;
 cleanup:
 return ret;
-- 
1.7.11.7

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


[libvirt] [PATCH 08/10] Check and handle error for virAsprintf() calls.

2013-01-03 Thread John Ferlan
---
 tests/xml2vmxtest.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c
index 653ab6c..c578109 100644
--- a/tests/xml2vmxtest.c
+++ b/tests/xml2vmxtest.c
@@ -193,8 +193,9 @@ testFormatVMXFileName(const char *src, void *opaque 
ATTRIBUTE_UNUSED)
 directoryAndFileName += strspn(directoryAndFileName,  );
 }
 
-virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName,
-directoryAndFileName);
+if (virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName,
+directoryAndFileName)  0)
+goto cleanup;
 } else if (STRPREFIX(src, /)) {
 /* Found absolute path */
 absolutePath = strdup(src);
-- 
1.7.11.7

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


[libvirt] [PATCH 05/10] Check return status for setting TCP_NODELAY option and generate a VIR_DEBUG message on failure. Allow connection to continue.

2013-01-03 Thread John Ferlan
---
 src/xen/xend_internal.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 84a25e8..1f779b0 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -91,8 +91,10 @@ do_connect(virConnectPtr xend)
 /*
  * try to desactivate slow-start
  */
-setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start,
-   sizeof(no_slow_start));
+if (setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start,
+   sizeof(no_slow_start))  0) {
+VIR_DEBUG(Unable to disable nagle algorithm);
+}
 
 
 if (connect(s, (struct sockaddr *)priv-addr, priv-addrlen) == -1) {
-- 
1.7.11.7

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


[libvirt] [PATCH 10/10] Check and handle select() errors from waitsocket().

2013-01-03 Thread John Ferlan
---
 src/phyp/phyp_driver.c | 42 --
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 8e26b0c..2dabd19 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -76,7 +76,6 @@ static int
 waitsocket(int socket_fd, LIBSSH2_SESSION * session)
 {
 struct timeval timeout;
-int rc;
 fd_set fd;
 fd_set *writefd = NULL;
 fd_set *readfd = NULL;
@@ -98,9 +97,7 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * session)
 if (dir  LIBSSH2_SESSION_BLOCK_OUTBOUND)
 writefd = fd;
 
-rc = select(socket_fd + 1, readfd, writefd, NULL, timeout);
-
-return rc;
+return select(socket_fd + 1, readfd, writefd, NULL, timeout);
 }
 
 /* this function is the layer that manipulates the ssh channel itself
@@ -131,7 +128,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
*exit_status,
 while ((channel = libssh2_channel_open_session(session)) == NULL 
libssh2_session_last_error(session, NULL, NULL, 0) ==
LIBSSH2_ERROR_EAGAIN) {
-waitsocket(sock, session);
+if (waitsocket(sock, session)  0  errno != EINTR) {
+virReportSystemError(errno, %s,
+ _(unable to wait on libssh2 socket));
+goto err;
+}
 }
 
 if (channel == NULL) {
@@ -140,7 +141,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
*exit_status,
 
 while ((rc = libssh2_channel_exec(channel, cmd)) ==
LIBSSH2_ERROR_EAGAIN) {
-waitsocket(sock, session);
+if (waitsocket(sock, session)  0  errno != EINTR) {
+virReportSystemError(errno, %s,
+ _(unable to wait on libssh2 socket));
+goto err;
+}
 }
 
 if (rc != 0) {
@@ -161,7 +166,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
*exit_status,
 /* this is due to blocking that would occur otherwise so we loop on
  * this condition */
 if (rc == LIBSSH2_ERROR_EAGAIN) {
-waitsocket(sock, session);
+if (waitsocket(sock, session)  0  errno != EINTR) {
+virReportSystemError(errno, %s,
+ _(unable to wait on libssh2 socket));
+goto err;
+}
 } else {
 break;
 }
@@ -170,7 +179,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
*exit_status,
 exitcode = 127;
 
 while ((rc = libssh2_channel_close(channel)) == LIBSSH2_ERROR_EAGAIN) {
-waitsocket(sock, session);
+if (waitsocket(sock, session)  0  errno != EINTR) {
+virReportSystemError(errno, %s,
+ _(unable to wait on libssh2 socket));
+goto err;
+}
 }
 
 if (rc == 0) {
@@ -735,7 +748,11 @@ phypUUIDTable_Pull(virConnectPtr conn)
 LIBSSH2_ERROR_EAGAIN) {
 goto err;
 } else {
-waitsocket(sock, session);
+if (waitsocket(sock, session)  0  errno != EINTR) {
+virReportSystemError(errno, %s,
+ _(unable to wait on libssh2 
socket));
+goto err;
+}
 }
 }
 } while (!channel);
@@ -769,7 +786,12 @@ phypUUIDTable_Pull(virConnectPtr conn)
 /* this is due to blocking that would occur otherwise
  * so we loop on this condition */
 
-waitsocket(sock, session);  /* now we wait */
+/* now we wait */
+if (waitsocket(sock, session)  0  errno != EINTR) {
+virReportSystemError(errno, %s,
+ _(unable to wait on libssh2 socket));
+goto err;
+}
 continue;
 }
 break;
-- 
1.7.11.7

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


[libvirt] [PATCH 01/10] Check and handle error for virAsprintf() calls.

2013-01-03 Thread John Ferlan
---
 src/interface/interface_backend_udev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/interface/interface_backend_udev.c 
b/src/interface/interface_backend_udev.c
index cc20b98..3231b73 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -654,9 +654,8 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name)
 ifacedef-data.bridge.stp = stp;
 
 /* Members of the bridge */
-virAsprintf(member_path, %s/%s,
-udev_device_get_syspath(dev), brif);
-if (!member_path) {
+if (virAsprintf(member_path, %s/%s,
+udev_device_get_syspath(dev), brif)  0) {
 virReportOOMError();
 goto cleanup;
 }
-- 
1.7.11.7

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


[libvirt] [PATCH 03/10] Check status when attempting to set SO_REUSEADDR flag on outgoing connection On failure, VIR_WARN(), but continue to connect.

2013-01-03 Thread John Ferlan
---
 src/rpc/virnetsocket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index ef93892..6684eef 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -470,7 +470,9 @@ int virNetSocketNewConnectTCP(const char *nodename,
 goto error;
 }
 
-setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt));
+if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt))  0) {
+VIR_WARN(Unable to enable port reuse);
+}
 
 if (connect(fd, runp-ai_addr, runp-ai_addrlen) = 0)
 break;
-- 
1.7.11.7

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


Re: [libvirt] [PATCH 01/10] Check and handle error for virAsprintf() calls.

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:13PM -0500, John Ferlan wrote:
 ---
  src/interface/interface_backend_udev.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
 diff --git a/src/interface/interface_backend_udev.c 
 b/src/interface/interface_backend_udev.c
 index cc20b98..3231b73 100644
 --- a/src/interface/interface_backend_udev.c
 +++ b/src/interface/interface_backend_udev.c
 @@ -654,9 +654,8 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name)
  ifacedef-data.bridge.stp = stp;
  
  /* Members of the bridge */
 -virAsprintf(member_path, %s/%s,
 -udev_device_get_syspath(dev), brif);
 -if (!member_path) {
 +if (virAsprintf(member_path, %s/%s,
 +udev_device_get_syspath(dev), brif)  0) {
  virReportOOMError();
  goto cleanup;
  }

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 02/10] Check and handle error for virAsprintf() calls. Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated.

2013-01-03 Thread Daniel P. Berrange
NB, you should restrict the first line of the commit message to
approx 70 characters. Then have a single blank line, followed
by the longer description. This ensures that you get sensible
subject lines :-)

On Thu, Jan 03, 2013 at 02:16:14PM -0500, John Ferlan wrote:
 ---
  src/parallels/parallels_storage.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
 diff --git a/src/parallels/parallels_storage.c 
 b/src/parallels/parallels_storage.c
 index e768d88..2908bee 100644
 --- a/src/parallels/parallels_storage.c
 +++ b/src/parallels/parallels_storage.c
 @@ -146,7 +146,7 @@ static char *parallelsMakePoolName(virConnectPtr conn, 
 const char *path)
  if (i == 0)
  name = strdup(path);
  else
 -virAsprintf(name, %s-%u, path, i);
 +ignore_value(virAsprintf(name, %s-%u, path, i));
  
  if (!name) {
  virReportOOMError();
 @@ -310,8 +310,7 @@ static int parallelsAddDiskVolume(virStoragePoolObjPtr 
 pool,
  if (VIR_ALLOC(def))
  goto no_memory;
  
 -virAsprintf(def-name, %s-%s, dom-def-name, diskName);
 -if (!def-name)
 +if (virAsprintf(def-name, %s-%s, dom-def-name, diskName)  0)
  goto no_memory;
  
  def-type = VIR_STORAGE_VOL_FILE;

ACK


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 05/10] Check return status for setting TCP_NODELAY option and generate a VIR_DEBUG message on failure. Allow connection to continue.

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:17PM -0500, John Ferlan wrote:
 ---
  src/xen/xend_internal.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index 84a25e8..1f779b0 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -91,8 +91,10 @@ do_connect(virConnectPtr xend)
  /*
   * try to desactivate slow-start
   */
 -setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start,
 -   sizeof(no_slow_start));
 +if (setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start,
 +   sizeof(no_slow_start))  0) {
 +VIR_DEBUG(Unable to disable nagle algorithm);
 +}

Hmm, I'd just make this  ignore_value() i think - it is something we
expect to fail with UNIX sockets

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 06/10] Check return on mkdir for LOCKSPACE_DIR

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:18PM -0500, John Ferlan wrote:
 ---
  tests/virlockspacetest.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/tests/virlockspacetest.c b/tests/virlockspacetest.c
 index 8673700..7678396 100644
 --- a/tests/virlockspacetest.c
 +++ b/tests/virlockspacetest.c
 @@ -293,7 +293,8 @@ static int testLockSpaceResourceLockPath(const void *args 
 ATTRIBUTE_UNUSED)
  
  lockspace = virLockSpaceNew(NULL);
  
 -mkdir(LOCKSPACE_DIR, 0700);
 +if (mkdir(LOCKSPACE_DIR, 0700)  0)
 +goto cleanup;
  
  if (virLockSpaceCreateResource(lockspace, LOCKSPACE_DIR /foo)  0)
  goto cleanup;

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 07/10] Check and handle error for virAsprintf() calls.

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:19PM -0500, John Ferlan wrote:
 ---
  tests/vmx2xmltest.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
 index 03a8989..2430350 100644
 --- a/tests/vmx2xmltest.c
 +++ b/tests/vmx2xmltest.c
 @@ -179,7 +179,9 @@ testParseVMXFileName(const char *fileName, void *opaque 
 ATTRIBUTE_UNUSED)
  goto cleanup;
  }
  
 -virAsprintf(src, [%s] %s, datastoreName, directoryAndFileName);
 +if (virAsprintf(src, [%s] %s, datastoreName,
 +directoryAndFileName)  0)
 +goto cleanup;
  } else if (STRPREFIX(fileName, /)) {
  /* Found absolute path referencing a file outside a datastore */
  src = strdup(fileName);
 @@ -188,7 +190,8 @@ testParseVMXFileName(const char *fileName, void *opaque 
 ATTRIBUTE_UNUSED)
  src = NULL;
  } else {
  /* Found single file name referencing a file inside a datastore */
 -virAsprintf(src, [datastore] directory/%s, fileName);
 +if (virAsprintf(src, [datastore] directory/%s, fileName)  0)
 +goto cleanup;
  }
  
cleanup:

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 08/10] Check and handle error for virAsprintf() calls.

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:20PM -0500, John Ferlan wrote:
 ---
  tests/xml2vmxtest.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c
 index 653ab6c..c578109 100644
 --- a/tests/xml2vmxtest.c
 +++ b/tests/xml2vmxtest.c
 @@ -193,8 +193,9 @@ testFormatVMXFileName(const char *src, void *opaque 
 ATTRIBUTE_UNUSED)
  directoryAndFileName += strspn(directoryAndFileName,  );
  }
  
 -virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName,
 -directoryAndFileName);
 +if (virAsprintf(absolutePath, /vmfs/volumes/%s/%s, datastoreName,
 +directoryAndFileName)  0)
 +goto cleanup;
  } else if (STRPREFIX(src, /)) {
  /* Found absolute path */
  absolutePath = strdup(src);

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 03/10] Check status when attempting to set SO_REUSEADDR flag on outgoing connection On failure, VIR_WARN(), but continue to connect.

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:15PM -0500, John Ferlan wrote:
 ---
  src/rpc/virnetsocket.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
 index ef93892..6684eef 100644
 --- a/src/rpc/virnetsocket.c
 +++ b/src/rpc/virnetsocket.c
 @@ -470,7 +470,9 @@ int virNetSocketNewConnectTCP(const char *nodename,
  goto error;
  }
  
 -setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt));
 +if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt))  0) 
 {
 +VIR_WARN(Unable to enable port reuse);
 +}
  
  if (connect(fd, runp-ai_addr, runp-ai_addrlen) = 0)
  break;

Hmm, not sure I agree with this. If this is something that should
not occurr, then we should virReportError. If it is something we
expect to occur, then VIR_WARN will annoy people with irrelevant
messages.

My inclination is to treat it as a fatal error

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 09/10] Ignore error returns for virBufferTrim().

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:21PM -0500, John Ferlan wrote:
 ---
  tools/virsh.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/tools/virsh.c b/tools/virsh.c
 index 283194a..9f834e4 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -544,7 +544,7 @@ vshTreePrintInternal(vshControl *ctl,
   false, indent)  0)
  goto cleanup;
  }
 -virBufferTrim(indent,   , -1);
 +ignore_value(virBufferTrim(indent,   , -1));
  
  /* If there was no child device, and we're the last in
   * a list of devices, then print another blank line */
 @@ -552,7 +552,7 @@ vshTreePrintInternal(vshControl *ctl,
  vshPrint(ctl, %s\n, virBufferCurrentContent(indent));
  
  if (!root)
 -virBufferTrim(indent, NULL, 2);
 +ignore_value(virBufferTrim(indent, NULL, 2));
  ret = 0;
  cleanup:
  return ret;

I must say I don't see the point in the return value
for virBufferTrim. I think I'd recommend turning it
into a 'void' function

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 10/10] Check and handle select() errors from waitsocket().

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:22PM -0500, John Ferlan wrote:
 ---
  src/phyp/phyp_driver.c | 42 --
  1 file changed, 32 insertions(+), 10 deletions(-)
 
 diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
 index 8e26b0c..2dabd19 100644
 --- a/src/phyp/phyp_driver.c
 +++ b/src/phyp/phyp_driver.c
 @@ -76,7 +76,6 @@ static int
  waitsocket(int socket_fd, LIBSSH2_SESSION * session)
  {
  struct timeval timeout;
 -int rc;
  fd_set fd;
  fd_set *writefd = NULL;
  fd_set *readfd = NULL;
 @@ -98,9 +97,7 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * session)
  if (dir  LIBSSH2_SESSION_BLOCK_OUTBOUND)
  writefd = fd;
  
 -rc = select(socket_fd + 1, readfd, writefd, NULL, timeout);
 -
 -return rc;
 +return select(socket_fd + 1, readfd, writefd, NULL, timeout);
  }
  
  /* this function is the layer that manipulates the ssh channel itself
 @@ -131,7 +128,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
 *exit_status,
  while ((channel = libssh2_channel_open_session(session)) == NULL 
 libssh2_session_last_error(session, NULL, NULL, 0) ==
 LIBSSH2_ERROR_EAGAIN) {
 -waitsocket(sock, session);
 +if (waitsocket(sock, session)  0  errno != EINTR) {
 +virReportSystemError(errno, %s,
 + _(unable to wait on libssh2 socket));
 +goto err;
 +}
  }
  
  if (channel == NULL) {
 @@ -140,7 +141,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
 *exit_status,
  
  while ((rc = libssh2_channel_exec(channel, cmd)) ==
 LIBSSH2_ERROR_EAGAIN) {
 -waitsocket(sock, session);
 +if (waitsocket(sock, session)  0  errno != EINTR) {
 +virReportSystemError(errno, %s,
 + _(unable to wait on libssh2 socket));
 +goto err;
 +}
  }
  
  if (rc != 0) {
 @@ -161,7 +166,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
 *exit_status,
  /* this is due to blocking that would occur otherwise so we loop on
   * this condition */
  if (rc == LIBSSH2_ERROR_EAGAIN) {
 -waitsocket(sock, session);
 +if (waitsocket(sock, session)  0  errno != EINTR) {
 +virReportSystemError(errno, %s,
 + _(unable to wait on libssh2 socket));
 +goto err;
 +}
  } else {
  break;
  }
 @@ -170,7 +179,11 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
 *exit_status,
  exitcode = 127;
  
  while ((rc = libssh2_channel_close(channel)) == LIBSSH2_ERROR_EAGAIN) {
 -waitsocket(sock, session);
 +if (waitsocket(sock, session)  0  errno != EINTR) {
 +virReportSystemError(errno, %s,
 + _(unable to wait on libssh2 socket));
 +goto err;
 +}
  }
  
  if (rc == 0) {
 @@ -735,7 +748,11 @@ phypUUIDTable_Pull(virConnectPtr conn)
  LIBSSH2_ERROR_EAGAIN) {
  goto err;
  } else {
 -waitsocket(sock, session);
 +if (waitsocket(sock, session)  0  errno != EINTR) {
 +virReportSystemError(errno, %s,
 + _(unable to wait on libssh2 
 socket));
 +goto err;
 +}
  }
  }
  } while (!channel);
 @@ -769,7 +786,12 @@ phypUUIDTable_Pull(virConnectPtr conn)
  /* this is due to blocking that would occur otherwise
   * so we loop on this condition */
  
 -waitsocket(sock, session);  /* now we wait */
 +/* now we wait */
 +if (waitsocket(sock, session)  0  errno != EINTR) {
 +virReportSystemError(errno, %s,
 + _(unable to wait on libssh2 socket));
 +goto err;
 +}
  continue;
  }
  break;

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 00/10] Resolve CHECKED_RETURN errors found by Coverity

2013-01-03 Thread Daniel P. Berrange
On Thu, Jan 03, 2013 at 02:16:12PM -0500, John Ferlan wrote:
 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=880388
 
 This set of patches resolves the CHECKED_RETURN (CWE-252) errors found
 by Coverity.
 
 
 John Ferlan (10):
   interface: Check and handle error for virAsprintf() calls.
   parallels: Check and handle error for virAsprintf() calls. Ignore the 
 return 
inparallelsMakePoolName() since subsequent check validates name
was allocated.
   rpc: Check status when attempting to set SO_REUSEADDR flag on outgoing
connection. On failure, VIR_WARN(), but continue to connect.
   vmware: Ignore the return status check for vmwareUpdateVMStatus in
convenience routine vmwareDomainObjListUpdateDomain
   xen: Check return status for setting TCP_NODELAY option and generate a
VIR_DEBUG message on failure. Allow connection to continue.
   virlockspacetest: Check return on mkdir for LOCKSPACE_DIR
   vmx2xmltest: Check and handle error for virAsprintf() calls.
   xml2vmxtest: Check and handle error for virAsprintf() calls.
   virsh: Ignore error returns for virBufferTrim().
   phyp: Check and handle select() errors from waitsocket().

There are a number of issues with vifAsprintf(). As a further patch I
think you should add ATTRIBUTE_RETURN_CHECK to this function, so we
see the problems immediately rather than relying on coverity.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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


Re: [libvirt] [PATCH 03/10] Check status when attempting to set SO_REUSEADDR flag on outgoing connection On failure, VIR_WARN(), but continue to connect.

2013-01-03 Thread John Ferlan
On 01/03/2013 02:34 PM, Daniel P. Berrange wrote:
 On Thu, Jan 03, 2013 at 02:16:15PM -0500, John Ferlan wrote:
 ---
  src/rpc/virnetsocket.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
 index ef93892..6684eef 100644
 --- a/src/rpc/virnetsocket.c
 +++ b/src/rpc/virnetsocket.c
 @@ -470,7 +470,9 @@ int virNetSocketNewConnectTCP(const char *nodename,
  goto error;
  }
  
 -setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt));
 +if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, opt, sizeof(opt))  
 0) {
 +VIR_WARN(Unable to enable port reuse);
 +}
  
  if (connect(fd, runp-ai_addr, runp-ai_addrlen) = 0)
  break;
 
 Hmm, not sure I agree with this. If this is something that should
 not occurr, then we should virReportError. If it is something we
 expect to occur, then VIR_WARN will annoy people with irrelevant
 messages.

I asked about this yesterday and Michal P responded. The REUSEADDR is a
more of a hint for connections, see the end of:


https://www.redhat.com/archives/libvir-list/2013-January/msg00064.html

I don't mind either way.

 
 My inclination is to treat it as a fatal error
 
 Daniel
 

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


Re: [libvirt] [PATCH 2/6] qemu: Add a hash table for the shared disks

2013-01-03 Thread Eric Blake
On 01/02/2013 07:37 AM, Osier Yang wrote:
 This introduces a hash table for qemu driver, to store the shared
 disk's info as (@major:minor, @ref_count). @ref_count is the number
 of domains which shares the disk.
 
 Since we only care about if the disk support unprivileged SG_IO
 commands, and the SG_IO commands only make sense for block disk,
 this patch only manages (add/remove hash entry) the shared disk for
 block disk.
 
 * src/qemu/qemu_conf.h: (Add member 'sharedDisks' of type
  virHashTablePtr; Declare helpers
  qemuGetSharedDiskKey, qemuAddSharedDisk
  and qemuRemoveSharedDisk)
 * src/qemu/qemu_conf.c (Implement the 3 helpers)
 * src/qemu/qemu_process.c (Update 'sharedDisks' when domain
starting and shutdown)
 * src/qemu/qemu_driver.c (Update 'sharedDisks' when attaching
   or detaching disk).
 ---
  src/qemu/qemu_conf.c|   86 
 +++
  src/qemu/qemu_conf.h|   12 ++
  src/qemu/qemu_driver.c  |   22 
  src/qemu/qemu_process.c |   15 
  4 files changed, 135 insertions(+), 0 deletions(-)

ACK, as I'd like to see this go in sooner rather than later, and what
you have works.  However...

 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index c6deb10..8440247 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -552,3 +552,89 @@ qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver,
  
  virHashForEach(driver-closeCallbacks, qemuDriverCloseCallbackRun, 
 data);
  }
 +
 +/* Construct the hash key for sharedDisks as major:minor */
 +char *
 +qemuGetSharedDiskKey(const char *disk_path)
 +{

Why are we converting major and minor into a malloc'd char*,...

 +int major, minor;

Here, I'd use 'maj' and 'min' to avoid any shadowing of the major() and
minor() macros.

 +int
 +qemuAddSharedDisk(virHashTablePtr sharedDisks,
 +  const char *disk_path)
 +{
 +size_t *ref = NULL;
 +char *key = NULL;
 +
 +if (!(key = qemuGetSharedDiskKey(disk_path)))
 +return -1;
 +
 +if ((ref = virHashLookup(sharedDisks, key))) {

...when you could just use a single int value comprising the combination
of major and minor as the hash key, if only you had used
virHashCreateFull() with custom comparators.  That would be more
efficient (no need to malloc each kay, comparisons are much faster as an
integer compare instead of a strcmp, and so on).  It may be worth a
followup patch that re-does the hash table to be more efficient.

 +++ b/src/qemu/qemu_driver.c
 @@ -843,6 +843,9 @@ qemuStartup(bool privileged,
  if ((qemu_driver-inactivePciHostdevs = pciDeviceListNew()) == NULL)
  goto error;
  
 +if (!(qemu_driver-sharedDisks = virHashCreate(30, NULL)))

Here's where the virHashCreateFull would let you avoid having to convert
the key into a string.

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



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

Re: [libvirt] [PATCH] qemu: fix a segfault in qemuProcessWaitForMonitor

2013-01-03 Thread Eric Blake
On 01/03/2013 11:15 AM, Ján Tomko wrote:
 Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in
 the case of QMP capability probing being used, leading to a segfault in
 strlen in the cleanup path.
 
 This patch opens the log and allocates the buffer if QMP probing was
 used, so we can display the helpful error message.
 ---
  src/qemu/qemu_process.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)

ACK.

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



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

Re: [libvirt] [PATCH] fwrite: silence __wur without using inline

2013-01-03 Thread Eric Blake
On 01/03/2013 11:16 AM, Paul Eggert wrote:
 Thanks for fixing that.  I'm still puzzled about
 why the problem happened with libvirt.

Why libvirt, and not detected elsewhere?  Probably because libvirt does
this at configure time:

AH_VERBATIM([FORTIFY_SOURCE],
[/* Enable compile-time and run-time bounds-checking, and some warnings,
without upsetting newer glibc. */
 #if !defined _FORTIFY_SOURCE  defined __OPTIMIZE__  __OPTIMIZE__
 # define _FORTIFY_SOURCE 2
 #endif

and the inline in question is only turned on when _FORTIFY_SOURCE is
enabled.  Packages where no one uses _FORTIFY_SOURCE by default would
never hit the issue.

 It's better now that
 stdio doesn't depend on extern-inline, but I worry that whatever
 bug the libvirt folks were having with stdio and extern inline
 might crop up when extern inline is used in other include files.

The elided code was not using _GL_EXTERN_INLINE, but _GL_INLINE.  They
have different semantics, but I'm hard-pressed to say _which_ semantics
are right from just those names.  If I understand correctly, we _want_
the semantics where the inline function is _always_ inlined and never
emitted as a linkable entry point, but I'd have to refer back to C99 and
the gcc manual to see which use of inline and possible __attribute__
combinations guarantees that point.  I've seen code that uses the name
ELIDABLE_INLINE for the specific combination we want used in header
files; maybe it's worth adding a name _GL_ELIDABLE_INLINE into
m4/extern-inline.m4 to make it easier to use the results.

 
 One minor improvement is that we can limit the workaround to
 glibc versions that have the problem, so I pushed this further
 change.

Thanks for that improvement.

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



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

Re: [libvirt] [PATCH] fwrite: silence __wur without using inline

2013-01-03 Thread Paul Eggert
On 01/03/13 12:23, Eric Blake wrote:

 The elided code was not using _GL_EXTERN_INLINE, but _GL_INLINE.  They
 have different semantics, but I'm hard-pressed to say _which_ semantics
 are right from just those names.

Standard C semantics.  _GL_EXTERN_INLINE is C99/C11 extern inline.
_GL_INILNE is C99/C11 inline.

 If I understand correctly, we _want_
 the semantics where the inline function is _always_ inlined and never
 emitted as a linkable entry point, but I'd have to refer back to C99 and
 the gcc manual to see which use of inline and possible __attribute__
 combinations guarantees that point.

Can't be done in Standard C, as far as I know.
With GNU C it can be done with __attribute__((__always_inline__)).

Why is it important that it not be a linkable entry point?

 maybe it's worth adding a name _GL_ELIDABLE_INLINE into
 m4/extern-inline.m4 to make it easier to use the results.

Sorry, I guess I don't see how this would work.
That is, I can see how _GL_ELIDABLE_INLINE would expand to 
'inline __attribute__((__always_inline__))'
in GNU C, but what would it expand to in Standard C?
It can't be expanded to nothing, since you don't want
a linkable entry point, and it can't be expanded to 'static',
at least not in a header, because the resulting function
couldn't be called from an extern inline function.

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


Re: [libvirt] [PATCH] fwrite: silence __wur without using inline

2013-01-03 Thread Eric Blake
On 01/03/2013 01:48 PM, Paul Eggert wrote:
 Can't be done in Standard C, as far as I know.

Oh well, not worth it then.

 With GNU C it can be done with __attribute__((__always_inline__)).
 
 Why is it important that it not be a linkable entry point?

At least in the case libvirt was hitting, multiple files used fwrite,
which in turn meant multiple linkable entries for rpl_fwrite were
emitted when linking things together; but because they weren't marked
'static', the linker didn't like us.  If we are going to have a wrapper
inline function in a header, then we want to ensure that the wrapper
does not cause duplicate links.

 
 maybe it's worth adding a name _GL_ELIDABLE_INLINE into
 m4/extern-inline.m4 to make it easier to use the results.
 
 Sorry, I guess I don't see how this would work.
 That is, I can see how _GL_ELIDABLE_INLINE would expand to 
 'inline __attribute__((__always_inline__))'
 in GNU C, but what would it expand to in Standard C?
 It can't be expanded to nothing, since you don't want
 a linkable entry point, and it can't be expanded to 'static',
 at least not in a header, because the resulting function
 couldn't be called from an extern inline function.

Indeed, that's an annoying limitation.  Which goes to show that what we
did for fwrite (avoid inline altogether, and instead use GNU C instead
of Standard C, to get the workaround we wanted) is really all the best
we can do - we have to use a case-by-case analysis of WHY a wrapper is
in the header, and either pull the wrapper out of a header and into a
real function, or rely on compiler extensions for the cases where the
wrapper only makes sense for that compiler.

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



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

Re: [libvirt] [PATCH 3/6] docs: Add docs and rng schema for new XML tag sgio

2013-01-03 Thread Eric Blake
On 01/02/2013 07:37 AM, Osier Yang wrote:
 This introduces new XML tag sgio for disk, its valid values
 are filtered and unfiltered, setting it as filtered will
 set the disk's unpriv_sgio to 0, and unfiltered to set it
 as 1, which allows the unprivileged SG_IO commands.
 ---
  docs/formatdomain.html.in |   14 ++-
  docs/schemas/domaincommon.rng |   54 +++-
  2 files changed, 49 insertions(+), 19 deletions(-)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 94df6f8..5e37b92 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -1395,7 +1395,19 @@
  rawio='yes', rawio capability will be enabled for all disks in
  the domain (because, in the case of QEMU, this capability can
  only be set on a per-process basis). This attribute is only
 -valid when device is lun.
 +valid when device is lun. NB, coderawio/code intends to
 +confine the capability per-device, however, current QEMU
 +implementation gives the domain process broader capability
 +than that (per-process basis, affects all the domain disks).
 +To confine the capability as much as possible for QEMU driver
 +as this stage, codesgio/code is recommended, it's more
 +secure than coderawio/code.
 +The optional codesgio/code attribute indicates whether the

For consistency with how we did it for 'rawio':

The optional codesgio/code attribute (span class=sincesince
1.0.2/span) indicates...

 +kernel will filter unprivileged SG_IO commands for the disk,
 +valid settings are filtered or unfiltered. Defaults to
 +filtered. Same with coderawio/code, codesgio/code

s/Same with/Similar to/

 +is only valid for device 'lun'.
 +span class=sincesince 1.0.2/span

...then drop the span here.

 +group
 +  attribute name=device
 +choice
 +  valuelun/value
 +/choice

Technically, the choice isn't needed here (but it doesn't hurt either).

ACK with the grammar cleaned up.

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



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

Re: [libvirt] [PATCH 02/10] Check and handle error for virAsprintf() calls. Ignore the return in parallelsMakePoolName() since subsequent check validates name was allocated.

2013-01-03 Thread Eric Blake
On 01/03/2013 12:32 PM, Daniel P. Berrange wrote:
 NB, you should restrict the first line of the commit message to
 approx 70 characters. Then have a single blank line, followed
 by the longer description. This ensures that you get sensible
 subject lines :-)

Also, using the same subject line across multiple patches can cause
grief to backporting efforts.  Inserting a prefix to say which portion
of code is touched helps to both remove the ambiguity and make it easier
to see at a glance why there are multiple patches.  I've modified the
commit message to:

parallels: check and handle error for virAsprintf() calls

Ignore the return in parallelsMakePoolName() since subsequent check
validates name was allocated.

 ACK

and I've now pushed 1 and 2, and will continue reading through the
series.  Congratulations on your first libvirt patch.

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



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

[libvirt] libvirt unable to start domains

2013-01-03 Thread Zeeshan Ali (Khattak)
Hi,
  I updated my libvirt git clone and install yesterday (after about a
month or so) and now I can't start domains:

$ virsh start fedora18
error: Failed to start domain fedora18
error: unsupported configuration: This QEMU does not support QXL
graphics adapters

While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
Fedora 18 repo. Just to be clear, qemu was not updated before the
problem appeared, only libvirt.

Here is the libvirtd log:
http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

Also attaching the domain config.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124
domain type='kvm'
  namefedora18/name
  uuidd64e1338-da51-c2df-578d-12db0641cde6/uuid
  titleFedora 18/title
  metadata
boxes:gnome-boxes xmlns:boxes=http://live.gnome.org/Boxes/;
  os-stateinstalled/os-state
  os-idhttp://fedoraproject.org/fedora/18/os-id
  media-idhttp://fedoraproject.org/fedora/18:1/media-id
/boxes:gnome-boxes
  /metadata
  memory unit='KiB'1179648/memory
  currentMemory unit='KiB'1179648/currentMemory
  vcpu placement='static'4/vcpu
  os
type arch='x86_64' machine='pc-1.2'hvm/type
boot dev='hd'/
  /os
  features
acpi/
apic/
  /features
  cpu mode='host-model'
model fallback='allow'/
topology sockets='1' cores='2' threads='2'/
  /cpu
  clock offset='utc'
timer name='rtc' tickpolicy='catchup'/
timer name='pit' tickpolicy='delay'/
  /clock
  on_poweroffdestroy/on_poweroff
  on_rebootrestart/on_reboot
  on_crashdestroy/on_crash
  pm
suspend-to-mem enabled='no'/
suspend-to-disk enabled='no'/
  /pm
  devices
emulator/usr/bin/qemu-kvm/emulator
disk type='file' device='disk'
  driver name='qemu' type='qcow2' cache='none'/
  source file='/home/zeenix/.local/share/gnome-boxes/images/fedora18'/
  target dev='vda' bus='virtio'/
  address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/
/disk
disk type='file' device='cdrom'
  driver name='qemu' type='raw'/
  target dev='hdc' bus='ide'/
  readonly/
  address type='drive' controller='0' bus='1' target='0' unit='0'/
/disk
controller type='usb' index='0'
  address type='pci' domain='0x' bus='0x00' slot='0x01' function='0x2'/
/controller
controller type='ide' index='0'
  address type='pci' domain='0x' bus='0x00' slot='0x01' function='0x1'/
/controller
controller type='virtio-serial' index='0'
  address type='pci' domain='0x' bus='0x00' slot='0x05' function='0x0'/
/controller
interface type='user'
  mac address='52:54:00:93:de:4b'/
  address type='pci' domain='0x' bus='0x00' slot='0x03' function='0x0'/
/interface
serial type='pty'
  target port='0'/
/serial
console type='pty'
  target type='serial' port='0'/
/console
channel type='spicevmc'
  target type='virtio' name='com.redhat.spice.0'/
  address type='virtio-serial' controller='0' bus='0' port='1'/
/channel
input type='tablet' bus='usb'/
input type='mouse' bus='ps2'/
graphics type='spice' autoport='yes'/
sound model='ac97'
  address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/
/sound
video
  model type='qxl' vram='65536' heads='1'/
  address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/
/video
memballoon model='virtio'
  address type='pci' domain='0x' bus='0x00' slot='0x08' function='0x0'/
/memballoon
  /devices
/domain

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

[libvirt] [PATCH] Install virtlockd.{socket, service} non executable

2013-01-03 Thread Guido Günther
since they're not scripts but systemd service files.
---
 src/Makefile.am |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index f7a9b91..0cfc1ed 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1687,9 +1687,9 @@ DISTCLEANFILES += virtlockd.service virtlockd.socket
 
 install-systemd: virtlockd.service virtlockd.socket install-sysconfig
$(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
-   $(INSTALL_SCRIPT) virtlockd.service \
+   $(INSTALL_DATA) virtlockd.service \
  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/
-   $(INSTALL_SCRIPT) virtlockd.socket \
+   $(INSTALL_DATA) virtlockd.socket \
  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/
 
 uninstall-systemd: uninstall-sysconfig
-- 
1.7.10.4

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


Re: [libvirt] [PATCH] Install virtlockd.{socket, service} non executable

2013-01-03 Thread Eric Blake
On 01/03/2013 03:05 PM, Guido Günther wrote:
 since they're not scripts but systemd service files.
 ---
  src/Makefile.am |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

ACK.

 
 diff --git a/src/Makefile.am b/src/Makefile.am
 index f7a9b91..0cfc1ed 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -1687,9 +1687,9 @@ DISTCLEANFILES += virtlockd.service virtlockd.socket
  
  install-systemd: virtlockd.service virtlockd.socket install-sysconfig
   $(MKDIR_P) $(DESTDIR)$(SYSTEMD_UNIT_DIR)
 - $(INSTALL_SCRIPT) virtlockd.service \
 + $(INSTALL_DATA) virtlockd.service \
 $(DESTDIR)$(SYSTEMD_UNIT_DIR)/
 - $(INSTALL_SCRIPT) virtlockd.socket \
 + $(INSTALL_DATA) virtlockd.socket \
 $(DESTDIR)$(SYSTEMD_UNIT_DIR)/
  
  uninstall-systemd: uninstall-sysconfig
 

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



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

[libvirt] [PATCH] The -vga CLI arg accepts qxl in qemu 1.2+ always

2013-01-03 Thread Doug Goldstein
The -vga command always accepts qxl in 1.2 and newer.
---
 src/qemu/qemu_capabilities.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f49a31c..c3ab488 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2282,6 +2282,7 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
 qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG);
 qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE);
 qemuCapsSet(caps, QEMU_CAPS_SECCOMP_SANDBOX);
+qemuCapsSet(caps, QEMU_CAPS_VGA_QXL);
 }
 
 
-- 
1.7.8.6

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


Re: [libvirt] libvirt unable to start domains

2013-01-03 Thread Doug Goldstein
On Thu, Jan 3, 2013 at 3:44 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:
 Hi,
   I updated my libvirt git clone and install yesterday (after about a
 month or so) and now I can't start domains:

 $ virsh start fedora18
 error: Failed to start domain fedora18
 error: unsupported configuration: This QEMU does not support QXL
 graphics adapters

 While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
 Fedora 18 repo. Just to be clear, qemu was not updated before the
 problem appeared, only libvirt.

 Here is the libvirtd log:
 http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

 Also attaching the domain config.

 --
 Regards,

 Zeeshan Ali (Khattak)
 FSF member#5124


Try the patch I sent in response to your original message. Not sure if
that's the 100% fix we want. Need to check if we should also be
checking for SPICE support in qemu_command.c but it should at the very
least get you going for now.

-- 
Doug Goldstein

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


Re: [libvirt] libvirt unable to start domains

2013-01-03 Thread Zeeshan Ali (Khattak)
On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:
 Hi,
   I updated my libvirt git clone and install yesterday (after about a
 month or so) and now I can't start domains:

 $ virsh start fedora18
 error: Failed to start domain fedora18
 error: unsupported configuration: This QEMU does not support QXL
 graphics adapters

 While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
 Fedora 18 repo. Just to be clear, qemu was not updated before the
 problem appeared, only libvirt.

 Here is the libvirtd log:
 http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

 Also attaching the domain config.

If my `git bisect run` script worked, I've found the commit
introducing this issue:

4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit
commit 4c993d8ab5473334e9d4155faa913476ada39069
Author: Guannan Ren g...@redhat.com
Date:   Fri Dec 14 15:06:31 2012 +0800

qemu: add qemu vga devices caps and one cap to mark them usable

QEMU_CAPS_DEVICE_QXL  -device qxl
QEMU_CAPS_DEVICE_VGA  -device VGA
QEMU_CAPS_DEVICE_CIRRUS_VGA   -device cirrus-vga
QEMU_CAPS_DEVICE_VMWARE_SVGA  -device vmware-svga

QEMU_CAPS_DEVICE_VIDEO_PRIMARY  /* safe to use -device XXX
 for primary video device */

Fix a typo in qemuCapsObjectTypes, the string 'qxl' here
should be -device qxl rather than -vga [...|qxl|..]

:04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758
5afc50956ad09cf6e740ce75800d746a53513271 M  src
:04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e
fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M  tests
bisect run success

FWIW, problem is not reproducible against the parent commit
34ca5684970fc5e4be0ec29809b73dea7be2d84f.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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


Re: [libvirt] [PATCH] fwrite: silence __wur without using inline

2013-01-03 Thread Paul Eggert
On 01/03/2013 01:00 PM, Eric Blake wrote:

 in the case libvirt was hitting, multiple files used fwrite,
 which in turn meant multiple linkable entries for rpl_fwrite were
 emitted when linking things together; but because they weren't marked
 'static', the linker didn't like us.

OK, but surely this problem would happen with any extern symbol
that gnulib defines.  I.e., it's not a problem specific to
extern inline functions; it's a problem with externs in general.

Isn't the right way to handle this, from the gnulib point of view,
the lib-symbol-visibility module?  Shouldn't libvirt be using that?
If libvirt was compiled with -fvisibility=hidden then any miscellanous
extern functions that it defined would be local to the libvirt .so,
and wouldn't clash with similarly-named functions in other shared objects.

 what we
 did for fwrite (avoid inline altogether, and instead use GNU C instead
 of Standard C, to get the workaround we wanted) is really all the best
 we can do

I hope that's not the case, because it pretty much means
that there's no portable way to use inline functions in
gnulib headers that contribute to shared objects.

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


Re: [libvirt] libvirt unable to start domains

2013-01-03 Thread Zeeshan Ali (Khattak)
On Fri, Jan 4, 2013 at 1:44 AM, Doug Goldstein car...@gentoo.org wrote:
 On Thu, Jan 3, 2013 at 3:44 PM, Zeeshan Ali (Khattak)
 zeesha...@gnome.org wrote:
 Hi,
   I updated my libvirt git clone and install yesterday (after about a
 month or so) and now I can't start domains:

 $ virsh start fedora18
 error: Failed to start domain fedora18
 error: unsupported configuration: This QEMU does not support QXL
 graphics adapters

 While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
 Fedora 18 repo. Just to be clear, qemu was not updated before the
 problem appeared, only libvirt.

 Here is the libvirtd log:
 http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

 Also attaching the domain config.

 --
 Regards,

 Zeeshan Ali (Khattak)
 FSF member#5124


 Try the patch I sent in response to your original message. Not sure if
 that's the 100% fix we want. Need to check if we should also be
 checking for SPICE support in qemu_command.c but it should at the very
 least get you going for now.

Thanks, I'll try that.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

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


Re: [libvirt] [PATCH 4/6] conf: Parse and format the new XML

2013-01-03 Thread Eric Blake
On 01/02/2013 07:37 AM, Osier Yang wrote:
 Like rawio, sgio is only allowed for block disk of device
 type lun.
 
 It doesn't default disk-sgio to filtered when parsing, as
 it won't be able to distinguish explicitly requested filtered
 and a default filtered in driver then. We have to error out for
 explicit request when the kernel doesn't support the new sysfs
 knob unpriv_sgio, however, for defaulted filtered, we can
 just ignore it if the kernel doesn't support unpriv_sgio.
 ---
  src/conf/domain_conf.c |   55 
 +++-
  src/conf/domain_conf.h |   10 
  ...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml |   32 +++
  tests/qemuxml2xmltest.c|1 +
  4 files changed, 85 insertions(+), 13 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml
 

 +enum virDomainDiskSGIO {
 +VIR_DOMAIN_DISK_SGIO_DEFAULT = 0,
 +VIR_DOMAIN_DISK_SGIO_FILTERED,
 +VIR_DOMAIN_DISK_SGIO_UNFILTERED,
 +
 +VIR_DOMAIN_DISK_SGIO_LAST
 +};
 +
  typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo;
  struct _virDomainBlockIoTuneInfo {
  unsigned long long total_bytes_sec;
 @@ -638,6 +646,7 @@ struct _virDomainDiskDef {
  virStorageEncryptionPtr encryption;
  bool rawio_specified;
  int rawio; /* no = 0, yes = 1 */

Don't know why we didn't make this 'bool', but that's pre-existing and
would be a separate cleanup patch.

 +int sgio;

I'd add /* enum virDomainDiskSGIO */, to make it easier to see what goes
in this int.

ACK.

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



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

Re: [libvirt] libvirt unable to start domains

2013-01-03 Thread Doug Goldstein
On Thu, Jan 3, 2013 at 5:58 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:
 On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak)
 zeesha...@gnome.org wrote:
 Hi,
   I updated my libvirt git clone and install yesterday (after about a
 month or so) and now I can't start domains:

 $ virsh start fedora18
 error: Failed to start domain fedora18
 error: unsupported configuration: This QEMU does not support QXL
 graphics adapters

 While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
 Fedora 18 repo. Just to be clear, qemu was not updated before the
 problem appeared, only libvirt.

 Here is the libvirtd log:
 http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

 Also attaching the domain config.

 If my `git bisect run` script worked, I've found the commit
 introducing this issue:

 4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit
 commit 4c993d8ab5473334e9d4155faa913476ada39069
 Author: Guannan Ren g...@redhat.com
 Date:   Fri Dec 14 15:06:31 2012 +0800

 qemu: add qemu vga devices caps and one cap to mark them usable

 QEMU_CAPS_DEVICE_QXL  -device qxl
 QEMU_CAPS_DEVICE_VGA  -device VGA
 QEMU_CAPS_DEVICE_CIRRUS_VGA   -device cirrus-vga
 QEMU_CAPS_DEVICE_VMWARE_SVGA  -device vmware-svga

 QEMU_CAPS_DEVICE_VIDEO_PRIMARY  /* safe to use -device XXX
  for primary video device */

 Fix a typo in qemuCapsObjectTypes, the string 'qxl' here
 should be -device qxl rather than -vga [...|qxl|..]

 :04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758
 5afc50956ad09cf6e740ce75800d746a53513271 M  src
 :04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e
 fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M  tests
 bisect run success

 FWIW, problem is not reproducible against the parent commit
 34ca5684970fc5e4be0ec29809b73dea7be2d84f.

 --
 Regards,

 Zeeshan Ali (Khattak)
 FSF member#5124


Basically for some reason we tried to build the command line up as
qemu-kvm ... -vga qxl instead of -device qxl-vga which is what I
thought Guannan was aiming for. I could be wrong though. I'll take
another look at it tonight.

-- 
Doug Goldstein

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


Re: [libvirt] [PATCH] The -vga CLI arg accepts qxl in qemu 1.2+ always

2013-01-03 Thread Eric Blake
On 01/03/2013 04:42 PM, Doug Goldstein wrote:
 The -vga command always accepts qxl in 1.2 and newer.

Are you sure?  I thought qemu ./configure allows a choice of which
graphics engines to support, and that you can compile qxl out (distros
don't do that, but a self-built qemu might support -vga but not QXL).  I
think that rather than blindly setting this cap, we need to find the
right QMP command to check which graphics engines are supported.

 ---
  src/qemu/qemu_capabilities.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
 index f49a31c..c3ab488 100644
 --- a/src/qemu/qemu_capabilities.c
 +++ b/src/qemu/qemu_capabilities.c
 @@ -2282,6 +2282,7 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
  qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG);
  qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE);
  qemuCapsSet(caps, QEMU_CAPS_SECCOMP_SANDBOX);
 +qemuCapsSet(caps, QEMU_CAPS_VGA_QXL);
  }
  
  
 

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



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

Re: [libvirt] RFC: An embedded mode for QEMU/LXC drivers

2013-01-03 Thread Eric Blake
On 01/02/2013 09:55 PM, Richard W.M. Jones wrote:
 On Wed, Jan 02, 2013 at 03:36:54PM +, Daniel P. Berrange wrote:
 This is something I was thinking about a little over the christmas
 break. I've no intention of implementing this in the immediate
 future, but wanted to post it while it was fresh in my mind.

 Historically we have had 2 ways of using the stateful drivers like
 QEMU/LXC/UML/etc.

  - system mode  - privileged libvirtd, one per host, started at boot
  - session mode - unprivileged libvirtd, one per non-root user, autostarted


 This leads me to wonder whether it is worth exploring the idea of a new
 type of libvirt connection.

  - embed mode - no libvirtd, driver runs in application context
 
 Seems like an excellent idea.

Seconded.  But I also have to wonder if Dan's work-in-progress on
fine-grain ACLs could also be used for the case of isolating domains
under the control of libguestfs so that virt-manager/oVirt/what-not
can't control the libguestfs domains, even though all the domains are
managed by the same libvirtd.  In other words, you may be able to
achieve embedded semantics by means of ACLs.

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



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

[libvirt] problem in pciFindStubDriver

2013-01-03 Thread Chunyan Liu
For Xen PV guests, pciback is the only working driver, pci-stub
doesn't work. Current function finds pci-stub driver first, if
pci-stub doesn't exist, find pciback. It won't work for Xen PV guests
since it will find pci-stub driver and return, but in fact it needs
pciback.

One way is to prefer pciback rather than pci-stub like in following
patch. But that will change the behaviour of other drivers too. Is
there any preferred aprroach to handle this?

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 5971764..c0a1c05 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -861,22 +861,22 @@ pciFindStubDriver(void)
 int probed = 0;

 recheck:
-if (pciDriverDir(drvpath, pci-stub)  0) {
+if (pciDriverDir(drvpath, pciback)  0) {
 return NULL;
 }

 if (virFileExists(drvpath)) {
 VIR_FREE(drvpath);
-return pci-stub;
+return pciback;
 }

-if (pciDriverDir(drvpath, pciback)  0) {
+if (pciDriverDir(drvpath, pci-stub)  0) {
 return NULL;
 }

 if (virFileExists(drvpath)) {
 VIR_FREE(drvpath);
-return pciback;
+return pci-stub;
 }

 VIR_FREE(drvpath);

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


Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu

2013-01-03 Thread li guang
在 2013-01-03四的 16:13 +0100,Ján Tomko写道:
 On 12/26/12 02:00, liguang wrote:
  @@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
* When QEMU grows support for  1 PCI domain, then pci.0 change
* to pciNN.0  where NN is the domain number
*/
  -if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS))
  +if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) {
  +virBufferAsprintf(buf, ,bus=pci-bridge%d, 
  info-addr.pci.bus);
  +} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) {
   virBufferAsprintf(buf, ,bus=pci.0);
 
 Is there any way (or plan) to use more pci buses with QEMU other than
 with the pci bridges? If not, we could just name the bridges pci.%d. (If
 we index the bridges from 1).

as far as I know, qemu can't use multi-pci-bus,
so only pci.0 accepted now.

 
  -else
  +} else {
   virBufferAsprintf(buf, ,bus=pci);
  +}
   if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
   virBufferAddLit(buf, ,multifunction=on);
   else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF)
  @@ -3455,6 +3460,32 @@ error:
   return NULL;
   }
 
 
   
  +char *
  +qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev,
  + qemuCapsPtr caps, int idx)
  +{
  +virBuffer buf = VIR_BUFFER_INITIALIZER;
  +
  +virBufferAsprintf(buf, pci-bridge,chassis_nr=1);
 
 The chassis number has to be unique for each bridge.

chassis number is not so important,
here, I just set all bridges in same chassis.

 
  +
  +if ((dev-type != VIR_DOMAIN_PCIBRIDGE_TYPE_ROOT) 
  +(qemuBuildDeviceAddressStr(buf, dev-info, caps)  0))
  +goto error;
  +else
  +virBufferAsprintf(buf, ,id=pci-bridge%d , idx);
  +
  +if (virBufferError(buf)) {
  +virReportOOMError();
  +goto error;
  +}
  +
  +return virBufferContentAndReset(buf);
  +
  +error:
  +virBufferFreeAndReset(buf);
  +return NULL;
  +
  +}
 

-- 
regards!
li guang


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

Re: [libvirt] [PATCH 4/6] conf: Parse and format the new XML

2013-01-03 Thread Osier Yang

On 2013年01月04日 08:21, Eric Blake wrote:

On 01/02/2013 07:37 AM, Osier Yang wrote:

Like rawio, sgio is only allowed for block disk of device
type lun.

It doesn't default disk-sgio to filtered when parsing, as
it won't be able to distinguish explicitly requested filtered
and a default filtered in driver then. We have to error out for
explicit request when the kernel doesn't support the new sysfs
knob unpriv_sgio, however, for defaulted filtered, we can
just ignore it if the kernel doesn't support unpriv_sgio.
---
  src/conf/domain_conf.c |   55 +++-
  src/conf/domain_conf.h |   10 
  ...qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml |   32 +++
  tests/qemuxml2xmltest.c|1 +
  4 files changed, 85 insertions(+), 13 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-sgio.xml




+enum virDomainDiskSGIO {
+VIR_DOMAIN_DISK_SGIO_DEFAULT = 0,
+VIR_DOMAIN_DISK_SGIO_FILTERED,
+VIR_DOMAIN_DISK_SGIO_UNFILTERED,
+
+VIR_DOMAIN_DISK_SGIO_LAST
+};
+
  typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo;
  struct _virDomainBlockIoTuneInfo {
  unsigned long long total_bytes_sec;
@@ -638,6 +646,7 @@ struct _virDomainDiskDef {
  virStorageEncryptionPtr encryption;
  bool rawio_specified;
  int rawio; /* no = 0, yes = 1 */


Don't know why we didn't make this 'bool', but that's pre-existing and
would be a separate cleanup patch.


+int sgio;


I'd add /* enum virDomainDiskSGIO */, to make it easier to see what goes
in this int.


Okay, will add it when pushing.



ACK.



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

Re: [libvirt] [PATCH 3/6] docs: Add docs and rng schema for new XML tag sgio

2013-01-03 Thread Osier Yang

On 2013年01月04日 05:25, Eric Blake wrote:

On 01/02/2013 07:37 AM, Osier Yang wrote:

This introduces new XML tag sgio for disk, its valid values
are filtered and unfiltered, setting it as filtered will
set the disk's unpriv_sgio to 0, and unfiltered to set it
as 1, which allows the unprivileged SG_IO commands.
---
  docs/formatdomain.html.in |   14 ++-
  docs/schemas/domaincommon.rng |   54 +++-
  2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 94df6f8..5e37b92 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1395,7 +1395,19 @@
  rawio='yes', rawio capability will be enabled for all disks in
  the domain (because, in the case of QEMU, this capability can
  only be set on a per-process basis). This attribute is only
-valid when device is lun.
+valid when device is lun. NB,coderawio/code  intends to
+confine the capability per-device, however, current QEMU
+implementation gives the domain process broader capability
+than that (per-process basis, affects all the domain disks).
+To confine the capability as much as possible for QEMU driver
+as this stage,codesgio/code  is recommended, it's more
+secure thancoderawio/code.
+The optionalcodesgio/code  attribute indicates whether the


For consistency with how we did it for 'rawio':

The optionalcodesgio/code  attribute (span class=sincesince
1.0.2/span) indicates...


Okay,




+kernel will filter unprivileged SG_IO commands for the disk,
+valid settings are filtered or unfiltered. Defaults to
+filtered. Same withcoderawio/code,codesgio/code


s/Same with/Similar to/


Okay,




+is only valid for device 'lun'.
+span class=sincesince 1.0.2/span


...then drop thespan  here.


+group
+attribute name=device
+choice
+valuelun/value
+/choice


Technically, thechoice  isn't needed here (but it doesn't hurt either).

ACK with the grammar cleaned up.


Will make the change when pushing, thanks.

Osier

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

Re: [libvirt] [PATCH 1/6] util: Prepare helpers for unpriv_sgio setting

2013-01-03 Thread Osier Yang

On 2013年01月03日 08:01, Eric Blake wrote:

On 01/02/2013 07:37 AM, Osier Yang wrote:

virGetDeviceID could be used across the sources, but it doesn't
relate with this series, and could be done later.

* src/util/virutil.h: (Declare virGetDeviceID, and
vir{Get,Set}DeviceUnprivSGIO)
* src/util/virutil.c: (Implement virGetDeviceID and
vir{Get,Set}DeviceUnprivSGIO)
* src/libvirt_private.syms: Export private symbols of upper helpers
---
  src/libvirt_private.syms |3 +
  src/util/virutil.c   |  140 ++
  src/util/virutil.h   |   11 
  3 files changed, 154 insertions(+), 0 deletions(-)


ACK.


+int
+virSetDeviceUnprivSGIO(const char *path,
+   const char *sysfs_dir,
+   int unpriv_sgio)
+{
+char *sysfs_path = NULL;
+char *val = NULL;
+int ret = -1;
+int rc;
+
+if (!(sysfs_path = virGetUnprivSGIOSysfsPath(path, sysfs_dir)))
+return -1;
+
+if (!virFileExists(sysfs_path)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(unpriv_sgio is not supported by this kernel));
+goto cleanup;


If 'unpriv_sgio' is 0 here, does it make the logic in any later patches
easier to return success in that case (you are setting things to the
default)?  But I'm okay with keeping it as a failure.



The unpriv_sgio == 0 could be requested explicitly by user (not only
the from the default), and in this case I'd think an error is better.
Otherwise I could see one will raise bug like sgio='filtered'
succeeded, but sgio='unfiltered' failed unless we document it
somewhere.

Regards,
Osier

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

Re: [libvirt] libvirt unable to start domains

2013-01-03 Thread Guannan Ren

On 01/04/2013 08:30 AM, Doug Goldstein wrote:

On Thu, Jan 3, 2013 at 5:58 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:

On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:

Hi,
   I updated my libvirt git clone and install yesterday (after about a
month or so) and now I can't start domains:

$ virsh start fedora18
error: Failed to start domain fedora18
error: unsupported configuration: This QEMU does not support QXL
graphics adapters

While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
Fedora 18 repo. Just to be clear, qemu was not updated before the
problem appeared, only libvirt.

Here is the libvirtd log:
http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

Also attaching the domain config.

If my `git bisect run` script worked, I've found the commit
introducing this issue:

4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit
commit 4c993d8ab5473334e9d4155faa913476ada39069
Author: Guannan Ren g...@redhat.com
Date:   Fri Dec 14 15:06:31 2012 +0800

 qemu: add qemu vga devices caps and one cap to mark them usable

 QEMU_CAPS_DEVICE_QXL  -device qxl
 QEMU_CAPS_DEVICE_VGA  -device VGA
 QEMU_CAPS_DEVICE_CIRRUS_VGA   -device cirrus-vga
 QEMU_CAPS_DEVICE_VMWARE_SVGA  -device vmware-svga

 QEMU_CAPS_DEVICE_VIDEO_PRIMARY  /* safe to use -device XXX
  for primary video device */

 Fix a typo in qemuCapsObjectTypes, the string 'qxl' here
 should be -device qxl rather than -vga [...|qxl|..]

:04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758
5afc50956ad09cf6e740ce75800d746a53513271 M  src
:04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e
fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M  tests
bisect run success

FWIW, problem is not reproducible against the parent commit
34ca5684970fc5e4be0ec29809b73dea7be2d84f.

--
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


Basically for some reason we tried to build the command line up as
qemu-kvm ... -vga qxl instead of -device qxl-vga which is what I
thought Guannan was aiming for. I could be wrong though. I'll take
another look at it tonight.



When libvirt reports this error, probably the qemu you are 
using doesn't support
qxl-vga as the primary video device, and at the same time the 
qemu doesn't

support -vga qxl either.

The reason why libvirt worked out before is that there is a 
typo for the caps flag bit

QEMU_CAPS_VGA_QXL fixed in the above commit. And libvirt didn't set
QEMU_CAPS_VGA_QXL on for qemu(1.2) by using QMP mode to check 
for qemu

capabilities.

  -{ qxl, QEMU_CAPS_VGA_QXL },
 +{ qxl, QEMU_CAPS_DEVICE_QXL },

 So Doug's patch probably could fix your problem temporarily, 
but we need to think it

 through as Eric said.

 Guannan




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

Re: [libvirt] [PATCH 2/6] qemu: Add a hash table for the shared disks

2013-01-03 Thread Osier Yang

On 2013年01月04日 04:02, Eric Blake wrote:

On 01/02/2013 07:37 AM, Osier Yang wrote:

This introduces a hash table for qemu driver, to store the shared
disk's info as (@major:minor, @ref_count). @ref_count is the number
of domains which shares the disk.

Since we only care about if the disk support unprivileged SG_IO
commands, and the SG_IO commands only make sense for block disk,
this patch only manages (add/remove hash entry) the shared disk for
block disk.

* src/qemu/qemu_conf.h: (Add member 'sharedDisks' of type
  virHashTablePtr; Declare helpers
  qemuGetSharedDiskKey, qemuAddSharedDisk
  and qemuRemoveSharedDisk)
* src/qemu/qemu_conf.c (Implement the 3 helpers)
* src/qemu/qemu_process.c (Update 'sharedDisks' when domain
starting and shutdown)
* src/qemu/qemu_driver.c (Update 'sharedDisks' when attaching
   or detaching disk).
---
  src/qemu/qemu_conf.c|   86 +++
  src/qemu/qemu_conf.h|   12 ++
  src/qemu/qemu_driver.c  |   22 
  src/qemu/qemu_process.c |   15 
  4 files changed, 135 insertions(+), 0 deletions(-)


ACK, as I'd like to see this go in sooner rather than later, and what
you have works.  However...



diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c6deb10..8440247 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -552,3 +552,89 @@ qemuDriverCloseCallbackRunAll(virQEMUDriverPtr driver,

  virHashForEach(driver-closeCallbacks, qemuDriverCloseCallbackRun,data);
  }
+
+/* Construct the hash key for sharedDisks as major:minor */
+char *
+qemuGetSharedDiskKey(const char *disk_path)
+{


Why are we converting major and minor into a malloc'd char*,...


+int major, minor;


Here, I'd use 'maj' and 'min' to avoid any shadowing of the major() and
minor() macros.


Oh, okay, I missed that.




+int
+qemuAddSharedDisk(virHashTablePtr sharedDisks,
+  const char *disk_path)
+{
+size_t *ref = NULL;
+char *key = NULL;
+
+if (!(key = qemuGetSharedDiskKey(disk_path)))
+return -1;
+
+if ((ref = virHashLookup(sharedDisks, key))) {


...when you could just use a single int value comprising the combination
of major and minor as the hash key, if only you had used


Sounds good, how about something like 805516, where 8 is major, and 
16 is the minor? It should be small enough to be not overflowing.



virHashCreateFull() with custom comparators.  That would be more
efficient (no need to malloc each kay, comparisons are much faster as an
integer compare instead of a strcmp, and so on).  It may be worth a
followup patch that re-does the hash table to be more efficient.


+++ b/src/qemu/qemu_driver.c
@@ -843,6 +843,9 @@ qemuStartup(bool privileged,
  if ((qemu_driver-inactivePciHostdevs = pciDeviceListNew()) == NULL)
  goto error;

+if (!(qemu_driver-sharedDisks = virHashCreate(30, NULL)))


Here's where the virHashCreateFull would let you avoid having to convert
the key into a string.



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

Re: [libvirt] [PATCH 2/3] add pci-brige device command line for qemu

2013-01-03 Thread Osier Yang

On 2013年01月04日 10:28, li guang wrote:

在 2013-01-03四的 16:13 +0100,Ján Tomko写道:

On 12/26/12 02:00, liguang wrote:

@@ -1801,10 +1803,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
   * When QEMU grows support for  1 PCI domain, then pci.0 change
   * to pciNN.0  where NN is the domain number
   */
-if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS))
+if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCIBRIDGE) {
+virBufferAsprintf(buf, ,bus=pci-bridge%d, info-addr.pci.bus);
+} else if (qemuCapsGet(caps, QEMU_CAPS_PCI_MULTIBUS)) {
  virBufferAsprintf(buf, ,bus=pci.0);


Is there any way (or plan) to use more pci buses with QEMU other than
with the pci bridges? If not, we could just name the bridges pci.%d. (If
we index the bridges from 1).


as far as I know, qemu can't use multi-pci-bus,
so only pci.0 accepted now.


At this point, I think it's better to ask the QEMU developers to
be involved in next series, to make sure things are right.






-else
+} else {
  virBufferAsprintf(buf, ,bus=pci);
+}
  if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
  virBufferAddLit(buf, ,multifunction=on);
  else if (info-addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_OFF)
@@ -3455,6 +3460,32 @@ error:
  return NULL;
  }





+char *
+qemuBuildPCIbridgeDevStr(virDomainPCIbridgeDefPtr dev,
+ qemuCapsPtr caps, int idx)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(buf, pci-bridge,chassis_nr=1);


The chassis number has to be unique for each bridge.


chassis number is not so important,
here, I just set all bridges in same chassis.




+
+if ((dev-type != VIR_DOMAIN_PCIBRIDGE_TYPE_ROOT)
+(qemuBuildDeviceAddressStr(buf,dev-info, caps)  0))
+goto error;
+else
+virBufferAsprintf(buf, ,id=pci-bridge%d , idx);
+
+if (virBufferError(buf)) {
+virReportOOMError();
+goto error;
+}
+
+return virBufferContentAndReset(buf);
+
+error:
+virBufferFreeAndReset(buf);
+return NULL;
+
+}






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

Re: [libvirt] libvirt unable to start domains

2013-01-03 Thread Guannan Ren

On 01/04/2013 08:30 AM, Doug Goldstein wrote:

On Thu, Jan 3, 2013 at 5:58 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:

On Thu, Jan 3, 2013 at 11:44 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:

Hi,
   I updated my libvirt git clone and install yesterday (after about a
month or so) and now I can't start domains:

$ virsh start fedora18
error: Failed to start domain fedora18
error: unsupported configuration: This QEMU does not support QXL
graphics adapters

While libvirt is self-built, I'm using qemu 1.2.2-1.fc18 from vanila
Fedora 18 repo. Just to be clear, qemu was not updated before the
problem appeared, only libvirt.

Here is the libvirtd log:
http://static.fi/~zeenix/tmp/libvirtd-session-zeenix.log (13 MB).

Also attaching the domain config.

If my `git bisect run` script worked, I've found the commit
introducing this issue:

4c993d8ab5473334e9d4155faa913476ada39069 is the first bad commit
commit 4c993d8ab5473334e9d4155faa913476ada39069
Author: Guannan Ren g...@redhat.com
Date:   Fri Dec 14 15:06:31 2012 +0800

 qemu: add qemu vga devices caps and one cap to mark them usable

 QEMU_CAPS_DEVICE_QXL  -device qxl
 QEMU_CAPS_DEVICE_VGA  -device VGA
 QEMU_CAPS_DEVICE_CIRRUS_VGA   -device cirrus-vga
 QEMU_CAPS_DEVICE_VMWARE_SVGA  -device vmware-svga

 QEMU_CAPS_DEVICE_VIDEO_PRIMARY  /* safe to use -device XXX
  for primary video device */

 Fix a typo in qemuCapsObjectTypes, the string 'qxl' here
 should be -device qxl rather than -vga [...|qxl|..]

:04 04 576dc9f1e74fae8b5d8c75349cc769e17c5f7758
5afc50956ad09cf6e740ce75800d746a53513271 M  src
:04 04 0b95704f11e9aacbd4c6510e29343a497c52c11e
fbd9445a795a2814d8739805ebaf9c7c2f0d4160 M  tests
bisect run success

FWIW, problem is not reproducible against the parent commit
34ca5684970fc5e4be0ec29809b73dea7be2d84f.

--
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


Basically for some reason we tried to build the command line up as
qemu-kvm ... -vga qxl instead of -device qxl-vga which is what I
thought Guannan was aiming for. I could be wrong though. I'll take
another look at it tonight.



Actually, qemu(=1.2) will use -device qxl-vga if it supports 
qxl-vga in its -device output
which means the QEMU_CAPS_DEVICE_QXL_VGA is set already. If qemu 
doesn't support
-device qxl-vga even though its version is above 1.2, it will use 
-vga qxl.


For qemu( 1.2), it definitely uses -vga qxl.

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


Re: [libvirt] [ESX:Error] HTTP 403 error for CURL upload operation

2013-01-03 Thread Ata Bohra



I was able to solve this issue, it seems for upload operation the CURL headers 
needs to contain:   Connection: Keep-Alive);  
  Content-Type: application/x-vnd.vmware-streamVmdk);
Content-Length: %ld
Though the error was related to forbidden access, but it seems it was not 
correct, with above set of headers and POST operation, VMDK uploads runs smooth 
:-).
Thanks!Ata
From: ata.hus...@hotmail.com
To: matthias.bo...@googlemail.com
Date: Tue, 1 Jan 2013 11:47:13 -0800
CC: libvir-list@redhat.com
Subject: Re: [libvirt] [ESX:Error] HTTP 403 error for CURL upload operation




 The URL /ha-nfc/52c6d592-7636-67c5-29f3-d5b373be4f42/disk-0.vmdk looks
 like the lease you get during OVA import on the ESX server to upload a
 disk image to, isn't it? I'm not sure why you would get and Forbidden
 error from the server here. Are you logged in with an restricted user
 account instead of the root account?

Yes, precicely this is what I am trying to do. The URL (deviceUrl) is obtained 
once the call to ImportVApp is successful, it does not import the disk on root 
directory but to the new VM directory created after the importvapp call. I 
remember trying this on ESXi4.1 and it used to work fine, ESXi5 seems to block 
it. I am logged in with root credentials. 
 
  To achieve this operation I've added a routine to support file Upload (for
  ESX driver as currently it only support buffer upload), I verified its
  functioning my uploading a file using datastore based URL:
  (http(s)//ip/file??dcPath=ha-datacenter?dsName=xxx).
 
 Are you trying to upload directly to the root of the datastore? IIRC
 this isn't allowed and you can only upload subdirectories in the
 datastore as in
 
 http(s)//ip/subdirectory/file??dcPath=ha-datacenter?dsName=xxx
 
OVA disk can be compressed (compression algorithm is identified as one of the 
fields inside creaimportspec result varible), also as OVA optimizes the disk we 
need to use this URL to upload the valid content and rest necessary messaging 
is done by server itself (say deflate the disk, uncompress it etc.).

 -- 
 Matthias Bolte
 http://photron.blogspot.com

  

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

Re: [libvirt] [PATCH] The -vga CLI arg accepts qxl in qemu 1.2+ always

2013-01-03 Thread Guannan Ren

On 01/04/2013 08:40 AM, Eric Blake wrote:

On 01/03/2013 04:42 PM, Doug Goldstein wrote:

The -vga command always accepts qxl in 1.2 and newer.

Are you sure?  I thought qemu ./configure allows a choice of which
graphics engines to support, and that you can compile qxl out (distros
don't do that, but a self-built qemu might support -vga but not QXL).  I
think that rather than blindly setting this cap, we need to find the
right QMP command to check which graphics engines are supported.



 When I tried qemu(1.3.50)
 ./configure --target-list=x86_64-softmmu --enable-debug 
--disable-spice


 #./x86_64-softmmu/qemu-system-x86_64 -h|grep -i qxl
  -vga [std|cirrus|vmware|qxl|xenfb|none]

 And it seem like no such QMP command for this on my qemu.

 Guannan




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