[libvirt] [PATCH 1/2] Fix AM_LDFLAGS typo

2013-09-01 Thread Guido Günther
---
This should probably go into 1.1.2

 src/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 636bcbc..19dfb81 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1455,7 +1455,7 @@ libvirt_driver_nwfilter_la_CFLAGS = \
-I$(top_srcdir)/src/access \
-I$(top_srcdir)/src/conf \
$(AM_CFLAGS)
-libvirt_driver_nwfilter_la_LDFLAGS = $(LD_AMFLAGS)
+libvirt_driver_nwfilter_la_LDFLAGS = $(AM_LDFLAGS)
 libvirt_driver_nwfilter_la_LIBADD = $(LIBPCAP_LIBS) $(LIBNL_LIBS) $(DBUS_LIBS)
 if WITH_DRIVER_MODULES
 libvirt_driver_nwfilter_la_LIBADD += ../gnulib/lib/libgnu.la
-- 
1.8.4.rc3

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


[libvirt] [PATCH 2/2] Pass AM_LDFLAGS to driver modules too

2013-09-01 Thread Guido Günther
This gives us a RO got, otherwise Debian's lintian complains:

W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_qemu.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_storage.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_uml.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_vbox.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_xen.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_nwfilter.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_storage.so
W: libvirt-bin: hardening-no-relro 
usr/lib/libvirt/connection-driver/libvirt_driver_uml.so
W: libvirt-sanlock: hardening-no-relro usr/lib/libvirt/lock-driver/sanlock.so
---
 src/Makefile.am | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 19dfb81..097682c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1000,7 +1000,7 @@ libvirt_driver_xen_la_LIBADD = libvirt_driver_xen_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_xen.la
 libvirt_driver_xen_la_LIBADD += ../gnulib/lib/libgnu.la
-libvirt_driver_xen_la_LDFLAGS = -module -avoid-version
+libvirt_driver_xen_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_xen.la
 # Stateful, so linked to daemon instead
@@ -1050,7 +1050,7 @@ libvirt_driver_vbox_la_LIBADD = 
libvirt_driver_vbox_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_vbox.la
 libvirt_driver_vbox_la_LIBADD += ../gnulib/lib/libgnu.la
-libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version
+libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_vbox.la
 # GPLv2-only license requries that it be linked into
@@ -1083,7 +1083,7 @@ libvirt_driver_libxl_la_LIBADD = 
libvirt_driver_libxl_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_libxl.la
 libvirt_driver_libxl_la_LIBADD += ../gnulib/lib/libgnu.la
-libvirt_driver_libxl_la_LDFLAGS = -module -avoid-version
+libvirt_driver_libxl_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_libxl.la
 # Stateful, so linked to daemon instead
@@ -1108,7 +1108,7 @@ libvirt_driver_qemu_la_LIBADD = 
libvirt_driver_qemu_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_qemu.la
 libvirt_driver_qemu_la_LIBADD += ../gnulib/lib/libgnu.la
-libvirt_driver_qemu_la_LDFLAGS = -module -avoid-version
+libvirt_driver_qemu_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_qemu.la
 # Stateful, so linked to daemon instead
@@ -1184,7 +1184,7 @@ libvirt_driver_uml_la_LIBADD = libvirt_driver_uml_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_uml.la
 libvirt_driver_uml_la_LIBADD += ../gnulib/lib/libgnu.la
-libvirt_driver_uml_la_LDFLAGS = -module -avoid-version
+libvirt_driver_uml_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_uml.la
 # Stateful, so linked to daemon instead
@@ -1361,7 +1361,7 @@ libvirt_driver_storage_la_LIBADD = 
libvirt_driver_storage_impl.la
 if WITH_DRIVER_MODULES
 mod_LTLIBRARIES += libvirt_driver_storage.la
 libvirt_driver_storage_la_LIBADD += ../gnulib/lib/libgnu.la
-libvirt_driver_storage_la_LDFLAGS = -module -avoid-version
+libvirt_driver_storage_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 else
 noinst_LTLIBRARIES += libvirt_driver_storage.la
 # Stateful, so linked to daemon instead
@@ -2114,7 +2114,7 @@ if WITH_SANLOCK
 lockdriver_LTLIBRARIES += sanlock.la
 sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES)
 sanlock_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS)
-sanlock_la_LDFLAGS = -module -avoid-version
+sanlock_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS)
 sanlock_la_LIBADD = -lsanlock_client \
../gnulib/lib/libgnu.la
 
-- 
1.8.4.rc3

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


Re: [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended

2013-09-01 Thread Gleb Natapov
On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones wrote:
 The comment in kvm_max_vcpus() states that it's using the recommended
 procedure from the kernel API documentation to get the max number
 of vcpus that kvm supports. It is, but by always returning the
 maximum number supported. The maximum number should only be used
 for development purposes. qemu should check KVM_CAP_NR_VCPUS for
 the recommended number of vcpus. This patch adds a warning if a user
 specifies a number of cpus between the recommended and max.
 
 v2:
 Incorporate tests for max_cpus, which specifies the maximum number
 of hotpluggable cpus. An additional note is that the message for
 the fail case was slightly changed, 'exceeds max cpus' to
 'exceeds the maximum cpus'. If this is unacceptable change for
 users like libvirt, then I'll need to spin a v3.
 
Looks good to me. Any ACKs, objections?

 Signed-off-by: Andrew Jones drjo...@redhat.com
 ---
  kvm-all.c | 69 
 ---
  1 file changed, 40 insertions(+), 29 deletions(-)
 
 diff --git a/kvm-all.c b/kvm-all.c
 index a2d49786365e3..021f5f47e53da 100644
 --- a/kvm-all.c
 +++ b/kvm-all.c
 @@ -1322,24 +1322,20 @@ static int kvm_irqchip_create(KVMState *s)
  return 0;
  }
  
 -static int kvm_max_vcpus(KVMState *s)
 +/* Find number of supported CPUs using the recommended
 + * procedure from the kernel API documentation to cope with
 + * older kernels that may be missing capabilities.
 + */
 +static int kvm_recommended_vcpus(KVMState *s)
  {
 -int ret;
 -
 -/* Find number of supported CPUs using the recommended
 - * procedure from the kernel API documentation to cope with
 - * older kernels that may be missing capabilities.
 - */
 -ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
 -if (ret) {
 -return ret;
 -}
 -ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
 -if (ret) {
 -return ret;
 -}
 +int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
 +return (ret) ? ret : 4;
 +}
  
 -return 4;
 +static int kvm_max_vcpus(KVMState *s)
 +{
 +int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
 +return (ret) ? ret : kvm_recommended_vcpus(s);
  }
  
  int kvm_init(void)
 @@ -1347,11 +1343,19 @@ int kvm_init(void)
  static const char upgrade_note[] =
  Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n
  (see http://sourceforge.net/projects/kvm).\n;
 +struct {
 +const char *name;
 +int num;
 +} num_cpus[] = {
 +{ SMP,  smp_cpus },
 +{ hotpluggable, max_cpus },
 +{ NULL, }
 +}, *nc = num_cpus;
 +int soft_vcpus_limit, hard_vcpus_limit;
  KVMState *s;
  const KVMCapabilityInfo *missing_cap;
  int ret;
  int i;
 -int max_vcpus;
  
  s = g_malloc0(sizeof(KVMState));
  
 @@ -1392,19 +1396,26 @@ int kvm_init(void)
  goto err;
  }
  
 -max_vcpus = kvm_max_vcpus(s);
 -if (smp_cpus  max_vcpus) {
 -ret = -EINVAL;
 -fprintf(stderr, Number of SMP cpus requested (%d) exceeds max cpus 
 -supported by KVM (%d)\n, smp_cpus, max_vcpus);
 -goto err;
 -}
 +/* check the vcpu limits */
 +soft_vcpus_limit = kvm_recommended_vcpus(s);
 +hard_vcpus_limit = kvm_max_vcpus(s);
  
 -if (max_cpus  max_vcpus) {
 -ret = -EINVAL;
 -fprintf(stderr, Number of hotpluggable cpus requested (%d) exceeds 
 max cpus 
 -supported by KVM (%d)\n, max_cpus, max_vcpus);
 -goto err;
 +while (nc-name) {
 +if (nc-num  soft_vcpus_limit) {
 +fprintf(stderr,
 +Warning: Number of %s cpus requested (%d) exceeds 
 +the recommended cpus supported by KVM (%d)\n,
 +nc-name, nc-num, soft_vcpus_limit);
 +
 +if (nc-num  hard_vcpus_limit) {
 +ret = -EINVAL;
 +fprintf(stderr, Number of %s cpus requested (%d) exceeds 
 +the maximum cpus supported by KVM (%d)\n,
 +nc-name, nc-num, hard_vcpus_limit);
 +goto err;
 +}
 +}
 +nc++;
  }
  
  s-vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
 -- 
 1.8.1.4

--
Gleb.

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


[libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-01 Thread Nehal J Wani
Define a new API virDomainInterfaceAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.

Define helper function virDomainInterfaceFree, which allows
the upper layer application to free the domain interface object
conveniently.

The API is going to provide multiple methods by flags, e.g.

  * Query guest agent
  * Parse lease file of dnsmasq
  * DHCP snooping

But at this stage, it will only work with guest agent, and flags
won't be supported.

include/libvirt/libvirt.h.in:
  * Define virDomainInterfaceAddresses, virDomainInterfaceFree
  * Define structs virDomainInterface, virDomainIPAddress

python/generator.py:
  * Skip the auto-generation for virDomainInterfaceAddresses
and virDomainInterfaceFree

src/driver.h:
  * Define domainInterfaceAddresses

src/libvirt.c:
  * Implement virDomainInterfaceAddresses
  * Implement virDomainInterfaceFree

src/libvirt_public.syms:
  * Export the new symbols

---
 include/libvirt/libvirt.h.in |  32 
 python/generator.py  |   3 ++
 src/driver.h |   6 +++
 src/libvirt.c| 115 +++
 src/libvirt_public.syms  |   6 +++
 5 files changed, 162 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a47e33c..1a34d02 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2044,6 +2044,38 @@ int virDomainGetInterfaceParameters 
(virDomainPtr dom,
  virTypedParameterPtr 
params,
  int *nparams, 
unsigned int flags);
 
+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 */
+unsigned int prefix; /* IP address prefix */
+};
+
+typedef struct _virDomainInterface virDomainInterface;
+typedef virDomainInterface *virDomainInterfacePtr;
+struct _virDomainInterface {
+char *name; /* interface name */
+char *hwaddr;   /* hardware address */
+unsigned int naddrs;/* number of items in @addrs */
+virDomainIPAddressPtr addrs;/* array of IP addresses */
+};
+
+int virDomainInterfaceAddresses(virDomainPtr dom,
+virDomainInterfacePtr **ifaces,
+unsigned int flags);
+
+void virDomainInterfaceFree(virDomainInterfacePtr iface);
+
 /* Management of domain block devices */
 
 int virDomainBlockPeek (virDomainPtr dom,
diff --git a/python/generator.py b/python/generator.py
index fb321c6..50f779b 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -458,6 +458,7 @@ skip_impl = (
 'virNodeGetMemoryParameters',
 'virNodeSetMemoryParameters',
 'virNodeGetCPUMap',
+'virDomainInterfaceAddresses',
 'virDomainMigrate3',
 'virDomainMigrateToURI3',
 )
@@ -560,6 +561,8 @@ skip_function = (
 virTypedParamsGetString,
 virTypedParamsGetUInt,
 virTypedParamsGetULLong,
+
+virDomainInterfaceFree, # Only useful in C
 )
 
 lxc_skip_function = (
diff --git a/src/driver.h b/src/driver.h
index be64333..eb4927b 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -518,6 +518,11 @@ typedef int
   unsigned int flags);
 
 typedef int
+(*virDrvDomainInterfaceAddresses)(virDomainPtr dom,
+  virDomainInterfacePtr **ifaces,
+  unsigned int flags);
+
+typedef int
 (*virDrvDomainMemoryStats)(virDomainPtr domain,
struct _virDomainMemoryStat *stats,
unsigned int nr_stats,
@@ -1238,6 +1243,7 @@ struct _virDriver {
 virDrvDomainInterfaceStats domainInterfaceStats;
 virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
 virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
+virDrvDomainInterfaceAddressesdomainInterfaceAddresses;
 virDrvDomainMemoryStats domainMemoryStats;
 virDrvDomainBlockPeek domainBlockPeek;
 virDrvDomainMemoryPeek domainMemoryPeek;
diff --git a/src/libvirt.c b/src/libvirt.c
index 07a3fd5..82c117f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -8643,6 +8643,96 @@ error:
 return -1;
 }
 
+ /**
+ * virDomainInterfaceAddresses:
+ * @dom: domain object
+ * @ifaces: pointer to an array of pointers pointing to interface objects
+ * @flags: 

[libvirt] [PATCHv5 0/5] Introduce API to query IP addresses for given domain

2013-09-01 Thread Nehal J Wani
This feature has been requested for a very long time. Since qemu guest
agent gives us reliable results, now the wait is over.

The RFC was first proposed by Michal Privoznik:
 http://www.redhat.com/archives/libvir-list/2012-February/msg00437.html
A patch was submitted, using structs:
 https://www.redhat.com/archives/libvir-list/2012-June/msg00220.html
Another patch was submitted, using XML:
 https://www.redhat.com/archives/libvir-list/2012-June/msg00904.html

Neither of the patches were accepted, probably due to lack of extensibility
and usability. Hence, we thought of using virTypedParameters for reporting
list of interfaces along with their MAC address and IP addresses. The RFC
can be found here:
 https://www.redhat.com/archives/libvir-list/2013-July/msg00084.html

The idea of extensibility was rejected and rendered out of scope of
libvirt. Hence, we were back to structs.

This API is called virDomainInterfaceAddresses which returns a dynamically
allocated array of virDomainInterface struct. The great disadvantage is
once this gets released, it's written in stone and we cannot change
or add an item into it.

The API supports two methods:

* Return information (list of all associated interfaces with MAC address
 and IP addresses) of all of the domain interfaces by default (if
 no interface name is provided)

* Return information for the specified interface (if an interface name
 is provided)

v5:
* s/virDomainInterfacesAddresses/virDomainInterfaceAddresses.
* Case for IP aliasing handled using virHashTable.
* New test cases added, involving multiple and 0 IP addresse(s)
  per interface.
* IP prefix changed from int to unsigned int.
* Changes to practice libvirt habits.

v4:
* Various style nits, indentation errors, memory leaks fixed.
* https://www.redhat.com/archives/libvir-list/2013-August/msg01265.html

v3:
* Upper bounds to number of interfaces and addresses per interface
  introduced.
* Change from array of structs to array of pointers
* ifaces_count moved from function argument to return value
* Changes in variable names
* Test cases added for qemuAgentGetInterfaces.
* https://www.redhat.com/archives/libvir-list/2013-August/msg01215.html

v2:
* Logical errors, memory leaks and few other errors fixed.
* https://www.redhat.com/archives/libvir-list/2013-August/msg00631.html

v1:
* http://www.redhat.com/archives/libvir-list/2013-July/msg01553.html

Nehal J Wani (5):
  domifaddr: Implement the public APIs
  domifaddr: Implement the remote protocol
  domifaddr: Implement the API for qemu
  domifaddr: Add virsh support
  domifaddr: Expose python binding

 daemon/remote.c | 131 +++
 examples/python/Makefile.am |   2 +-
 examples/python/README  |   1 +
 examples/python/domipaddrs.py   |  50 +++
 include/libvirt/libvirt.h.in|  32 +++
 python/generator.py |   3 +
 python/libvirt-override-api.xml |   8 +-
 python/libvirt-override.c   | 111 +++
 src/driver.h|   6 ++
 src/libvirt.c   | 115 
 src/libvirt_public.syms |   6 ++
 src/qemu/qemu_agent.c   | 193 
 src/qemu/qemu_agent.h   |   3 +
 src/qemu/qemu_driver.c  |  55 
 src/remote/remote_driver.c  |  99 +
 src/remote/remote_protocol.x|  40 -
 src/remote_protocol-structs |  24 +
 tests/qemuagenttest.c   | 149 +++
 tools/virsh-domain-monitor.c| 119 +
 tools/virsh.pod |  11 +++
 20 files changed, 1155 insertions(+), 3 deletions(-)
 create mode 100755 examples/python/domipaddrs.py

-- 
1.7.11.7

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


[libvirt] [PATCHv5 2/5] domifaddr: Implement the remote protocol

2013-09-01 Thread Nehal J Wani
daemon/remote.c
   * Define remoteSerializeDomainInterface, 
remoteDispatchDomainInterfaceAddresses

src/remote/remote_driver.c
   * Define remoteDomainInterfaceAddresses

src/remote/remote_protocol.x
   * New RPC procedure: REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES
   * Define structs remote_domain_ip_addr, remote_domain_interface,
 remote_domain_interfaces_addresse_args, 
remote_domain_interface_addresses_ret
   * Introduce upper bounds (to handle DDoS attacks):
 REMOTE_DOMAIN_INTERFACE_MAX = 2048
 REMOTE_DOMAIN_IP_ADDR_MAX = 2048
 Restrictions on the maximum number of aliases per interface were
 removed after kernel v2.0, and theoretically, at present, there
 are no upper limits on number of interfaces per virtual machine
 and on the number of IP addresses per interface.

src/remote_protocol-structs
   * New structs added

---
 daemon/remote.c  | 131 +++
 src/remote/remote_driver.c   |  99 
 src/remote/remote_protocol.x |  40 -
 src/remote_protocol-structs  |  24 
 4 files changed, 293 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 6ace7af..7091cab 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -5144,7 +5144,138 @@ cleanup:
 return rv;
 }
 
+static int
+remoteSerializeDomainInterface(virDomainInterfacePtr *ifaces,
+   unsigned int ifaces_count,
+   remote_domain_interface_addresses_ret *ret)
+{
+size_t i, j;
+
+if (ifaces_count  REMOTE_DOMAIN_INTERFACE_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Number of interfaces, %d exceeds the max limit: %d),
+   ifaces_count, REMOTE_DOMAIN_INTERFACE_MAX);
+return -1;
+}
+
+if (VIR_ALLOC_N(ret-ifaces.ifaces_val, ifaces_count)  0)
+return -1;
+
+ret-ifaces.ifaces_len = ifaces_count;
+
+for (i = 0; i  ifaces_count; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+remote_domain_interface *iface_ret = (ret-ifaces.ifaces_val[i]);
+
+if ((VIR_STRDUP(iface_ret-name, iface-name))  0)
+goto cleanup;
+
+if (iface-hwaddr) {
+char **hwaddr_p = NULL;
+if (VIR_ALLOC(hwaddr_p)  0)
+goto cleanup;
+if (VIR_STRDUP(*hwaddr_p, iface-hwaddr)  0) {
+VIR_FREE(hwaddr_p);
+goto cleanup;
+}
+
+iface_ret-hwaddr = hwaddr_p;
+}
+
+if (iface-naddrs  REMOTE_DOMAIN_IP_ADDR_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Number of interfaces, %d exceeds the max limit: 
%d),
+   iface-naddrs, REMOTE_DOMAIN_IP_ADDR_MAX);
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(iface_ret-addrs.addrs_val,
+iface-naddrs)  0)
+goto cleanup;
+
+iface_ret-addrs.addrs_len = iface-naddrs;
+
+for (j = 0; j  iface-naddrs; j++) {
+virDomainIPAddressPtr ip_addr = (iface-addrs[j]);
+remote_domain_ip_addr *ip_addr_ret =
+(iface_ret-addrs.addrs_val[j]);
+
+if (VIR_STRDUP(ip_addr_ret-addr, ip_addr-addr)  0)
+goto cleanup;
+
+ip_addr_ret-prefix = ip_addr-prefix;
+ip_addr_ret-type = ip_addr-type;
+}
+}
+
+return 0;
+
+cleanup:
+if (ret-ifaces.ifaces_val) {
+for (i = 0; i  ifaces_count; i++) {
+remote_domain_interface *iface_ret = (ret-ifaces.ifaces_val[i]);
+VIR_FREE(iface_ret-name);
+VIR_FREE(iface_ret-hwaddr);
+for (j = 0; j  iface_ret-addrs.addrs_len; j++) {
+remote_domain_ip_addr *ip_addr =
+(iface_ret-addrs.addrs_val[j]);
+VIR_FREE(ip_addr-addr);
+}
+VIR_FREE(iface_ret);
+}
+VIR_FREE(ret-ifaces.ifaces_val);
+}
 
+return -1;
+}
+
+static int
+remoteDispatchDomainInterfaceAddresses(
+virNetServerPtr server ATTRIBUTE_UNUSED,
+virNetServerClientPtr client,
+virNetMessagePtr msg ATTRIBUTE_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_domain_interface_addresses_args *args,
+remote_domain_interface_addresses_ret *ret)
+{
+size_t i;
+int rv = -1;
+virDomainPtr dom = NULL;
+virDomainInterfacePtr *ifaces = NULL;
+int ifaces_count = 0;
+struct daemonClientPrivate *priv =
+virNetServerClientGetPrivateData(client);
+
+if (!priv-conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(connection not open));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv-conn, args-dom)))
+goto cleanup;
+
+if ((ifaces_count = virDomainInterfaceAddresses(dom, ifaces, 
args-flags))  0)
+goto cleanup;
+
+if (remoteSerializeDomainInterface(ifaces, ifaces_count, 

[libvirt] [PATCHv5 4/5] domifaddr: Add virsh support

2013-09-01 Thread Nehal J Wani
Use virDomainInterfaceAddresses in virsh

tools/virsh-domain-monitor.c
   * Introduce new command : domifaddr
 Usage: domifaddr domain [interface] [full]

 Example output:
 virsh # domifaddr f18
 Name   MAC address  Protocol IP Address
 
---
 lo 00:00:00:00:00:00ipv4 127.0.0.1/8
 -  -ipv6 ::1/128
 eth0   52:54:00:2e:45:ceipv4 10.1.33.188/24
 -  -ipv6 2001:db8:0:f101::2/64
 -  -ipv6 fe80::5054:ff:fe2e:45ce/64
 eth1   52:54:00:b1:70:19ipv4 192.168.105.201/16
 -  -ipv4 192.168.201.195/16
 -  -ipv6 fe80::5054:ff:feb1:7019/64
 eth2   52:54:00:36:2a:e5

tools/virsh.pod
   * Document new command

---
 tools/virsh-domain-monitor.c | 119 +++
 tools/virsh.pod  |  11 
 2 files changed, 130 insertions(+)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index b29b82a..cd1df7a 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1871,6 +1871,119 @@ cleanup:
 }
 #undef FILTER
 
+/* domifaddr command
+ */
+static const vshCmdInfo info_domifaddr[] = {
+{help, N_(Get network interfaces' addresses for a running domain)},
+{desc, N_(Get network interfaces' addresses for a running domain)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_domifaddr[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{interface, VSH_OT_DATA, VSH_OFLAG_NONE, N_(network interface name)},
+{full, VSH_OT_BOOL, VSH_OFLAG_NONE, N_(display full fields)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+const char *interface = NULL;
+virDomainInterfacePtr *ifaces = NULL;
+size_t i, j;
+int ifaces_count = 0;
+unsigned int flags = 0;
+bool ret = false;
+bool optFull = vshCommandOptBool(cmd, full);
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptString(cmd, interface, interface)  0) {
+goto cleanup;
+}
+
+if ((ifaces_count = virDomainInterfaceAddresses(dom, ifaces, flags))  0) 
{
+vshError(ctl, _(Failed to query for interfaces addresses));
+goto cleanup;
+}
+
+vshPrintExtra(ctl,  %-10s %-20s %-8s %s\n%s%s\n, _(Name),
+  _(MAC address), _(Protocol), _(Address),
+  _(-),
+  _(--));
+
+for (i = 0; i  ifaces_count; i++) {
+virDomainInterfacePtr iface = ifaces[i];
+const char *hwaddr = ;
+const char *ip_addr_str = NULL;
+const char *type = NULL;
+
+if (interface  STRNEQ(interface, iface-name))
+continue;
+
+if (iface-hwaddr)
+hwaddr = iface-hwaddr;
+
+/* When the interface has no IP address */
+if (!iface-naddrs) {
+vshPrintExtra(ctl,  %-10s %-17s\n,
+  iface-name, hwaddr);
+}
+
+for (j = 0; j  iface-naddrs; j++) {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+switch (iface-addrs[j].type) {
+case VIR_IP_ADDR_TYPE_IPV4:
+type = ipv4;
+break;
+case VIR_IP_ADDR_TYPE_IPV6:
+type = ipv6;
+break;
+}
+
+virBufferAsprintf(buf, %-12s %s/%d,
+  type, iface-addrs[j].addr,
+  iface-addrs[j].prefix);
+
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError();
+return ret;
+}
+
+ip_addr_str = virBufferContentAndReset(buf);
+
+if (!ip_addr_str)
+ip_addr_str = ;
+
+/* Don't repeat interface name */
+if (optFull || !j)
+vshPrintExtra(ctl,  %-10s %-17s%s\n,
+  iface-name, hwaddr, ip_addr_str);
+else
+vshPrintExtra(ctl,  %-10s %-17s%s\n,
+  -,-,ip_addr_str);
+
+virBufferFreeAndReset(buf);
+}
+}
+
+ret = true;
+
+cleanup:
+if (ifaces)
+for (i = 0; i  ifaces_count; i++)
+virDomainInterfaceFree(ifaces[i]);
+VIR_FREE(ifaces);
+
+virDomainFree(dom);
+return ret;
+}
+
 const vshCmdDef domMonitoringCmds[] = {
 {.name = domblkerror,
  .handler = cmdDomBlkError,
@@ -1944,5 +2057,11 @@ const vshCmdDef domMonitoringCmds[] = {
  .info = info_list,
  .flags = 0
 },

[libvirt] [PATCHv5 5/5] domifaddr: Expose python binding

2013-09-01 Thread Nehal J Wani
Expose virDomainInterfaceAddresses to python binding

examples/python/Makefile.am:
  * Add new file domipaddrs.py

examples/python/README:
  * Add documentation for the python example

python/libvirt-override-api.xml:
  * Add new symbol for virDomainInterfaceAddresses

python/libvirt-override.c:
  * Hand written python api

Example:
  $ ./run python ./examples/python/domipaddrs.py f18
  Interface  MAC address  Protocol Address
  lo 00:00:00:00:00:00ipv4 127.0.0.1/8
  lo 00:00:00:00:00:00ipv6 ::1/128
  eth2   52:54:00:36:2a:e5
  eth1   52:54:00:b1:70:19ipv4 192.168.105.201/16
  eth1   52:54:00:b1:70:19ipv4 192.168.201.195/16
  eth1   52:54:00:b1:70:19ipv6 fe80::5054:ff:feb1:7019/64
  eth0   52:54:00:2e:45:ceipv4 10.1.33.188/24
  eth0   52:54:00:2e:45:ceipv6 2001:db8:0:f101::2/64
  eth0   52:54:00:2e:45:ceipv6 fe80::5054:ff:fe2e:45ce/64

---
 examples/python/Makefile.am |   2 +-
 examples/python/README  |   1 +
 examples/python/domipaddrs.py   |  50 ++
 python/libvirt-override-api.xml |   8 ++-
 python/libvirt-override.c   | 111 
 5 files changed, 170 insertions(+), 2 deletions(-)
 create mode 100755 examples/python/domipaddrs.py

diff --git a/examples/python/Makefile.am b/examples/python/Makefile.am
index 2cacfa1..d33ee17 100644
--- a/examples/python/Makefile.am
+++ b/examples/python/Makefile.am
@@ -17,4 +17,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..1285d52 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 along with their MAC 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..679e0bf
--- /dev/null
+++ b/examples/python/domipaddrs.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# domipaddrds - print domain interfaces along with their MAC and IP addresses
+
+import libvirt
+import sys
+
+def usage():
+print Usage: %s [URI] DOMAIN % sys.argv[0]
+print Print domain interfaces along with their MAC and IP 
addresses
+
+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.interfaceAddresses(0)
+if (ifaces == None):
+print Failed to get domain interfaces
+sys.exit(0)
+
+print  {0:10} {1:20} {2:12} {3}.format(Interface, MAC address, 
Protocol, Address)
+
+for (name, val) in ifaces.iteritems():
+if val['ip_addrs']:
+for addr in val['ip_addrs']:
+   print  {0:10} {1:19}.format(name, val['hwaddr']),
+   print  {0:12} {1}/{2} .format(addr['type'], addr['addr'], 
addr['prefix']),
+   print
+else:
+print  {0:10} {1:19}.format(name, val['hwaddr']),
+print
diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml
index 9a88215..60491de 100644
--- a/python/libvirt-override-api.xml
+++ b/python/libvirt-override-api.xml
@@ -602,5 +602,11 @@
   arg name='conn' type='virConnectPtr' info='pointer to the hypervisor 
connection'/
   arg name='flags' type='int' info='unused, pass 0'/
 /function
-  /symbols
+function name='virDomainInterfaceAddresses' file='python'
+  inforeturns a dictionary of domain interfaces along with their MAC and 
IP addresses/info
+  arg name='dom' type='virDomainPtr' info='pointer to the domain'/
+  arg name='flags' type='unsigned int' info='extra flags; not used yet, 
so callers should always pass 0'/
+  return type='virDomainInterfacePtr' info=dictionary of domain 
interfaces along with their MAC and IP addresses/
+/function
+/symbols
 /api
diff --git a/python/libvirt-override.c b/python/libvirt-override.c
index d16b9a2..67e0cb8 100644

[libvirt] [PATCHv5 3/5] domifaddr: Implement the API for qemu

2013-09-01 Thread Nehal J Wani
By querying the qemu guest agent with the QMP command
guest-network-get-interfaces and converting the received JSON
output to structured objects.

Although ifconfig is deprecated, IP aliases created by ifconfig
are supported by this API. The legacy syntax of an IP alias is:
ifname:alias-name. Since we want all aliases to be clubbed
under parent interface, simply stripping :alias-name suffices.
Note that IP aliases formed by ip aren't visible to ifconfig,
and aliases created by ip do not have any specific name. But
we are lucky, as qemuga detects aliases created by both.

src/qemu/qemu_agent.h:
  * Define qemuAgentGetInterfaces

src/qemu/qemu_agent.c:
  * Implement qemuAgentGetInterface

src/qemu/qemu_driver.c:
  * New function qemuDomainInterfaceAddresses

src/remote_protocol-sructs:
  * Define new structs

tests/qemuagenttest.c:
  * Add new test: testQemuAgentGetInterfaces
Test cases for IP aliases, 0 or multiple ipv4/ipv6 address(es)

---
 src/qemu/qemu_agent.c  | 193 +
 src/qemu/qemu_agent.h  |   3 +
 src/qemu/qemu_driver.c |  55 ++
 tests/qemuagenttest.c  | 149 ++
 4 files changed, 400 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 2cd0ccc..009ed77 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1320,6 +1320,199 @@ cleanup:
 }
 
 /*
+ * qemuAgentGetInterfaces:
+ * @mon: Agent
+ * @ifaces: pointer to an array of pointers pointing to interface objects
+ *
+ * Issue guest-network-get-interfaces to guest agent, which returns the
+ * list of a interfaces of a running domain along with their IP and MAC
+ * addresses.
+ *
+ * Returns: number of interfaces on success, -1 on error.
+ */
+int
+qemuAgentGetInterfaces(qemuAgentPtr mon,
+   virDomainInterfacePtr **ifaces)
+{
+int ret = -1;
+size_t i, j;
+int size = -1;
+virJSONValuePtr cmd = NULL;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr ret_array = NULL;
+size_t ifaces_count = 0;
+size_t addrs_count = 0;
+virDomainInterfacePtr *ifaces_ret = NULL;
+virHashTablePtr ifaces_store = NULL;
+
+/* Initially the bag of ifaces is empty */
+if (!(ifaces_store = virHashCreate(ifaces_count, NULL)))
+return -1;
+
+if (!(cmd = qemuAgentMakeCommand(guest-network-get-interfaces, NULL)))
+   return -1;
+
+if (qemuAgentCommand(mon, cmd, reply, 
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK)  0 ||
+qemuAgentCheckError(cmd, reply)  0) {
+goto cleanup;
+}
+
+if (!(ret_array = virJSONValueObjectGet(reply, return))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent didn't provide 'return' field));
+goto cleanup;
+}
+
+if ((size = virJSONValueArraySize(ret_array))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent didn't return an array of interfaces));
+goto cleanup;
+}
+
+for (i = 0; i  size; i++) {
+virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i);
+virJSONValuePtr ip_addr_arr = NULL;
+const char *hwaddr, *ifname_s, *name = NULL;
+char **ifname = NULL;
+int ip_addr_arr_size;
+virDomainInterfacePtr iface = NULL;
+
+/* Shouldn't happen but doesn't hurt to check neither */
+if (!tmp_iface) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(something has went really wrong));
+goto error;
+}
+
+/* interface name is required to be presented */
+name = virJSONValueObjectGetString(tmp_iface, name);
+if (!name) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(qemu agent didn't provide 'name' field));
+goto error;
+}
+
+/* Handle aliases of type ifname:alias-name */
+ifname = virStringSplit(name, :, 2);
+ifname_s = ifname[0];
+
+iface = virHashLookup(ifaces_store, ifname_s);
+
+/* If the storage bag doesn't contain this iface, add it */
+if (!iface) {
+if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1)  0)
+goto cleanup;
+
+if (VIR_ALLOC(ifaces_ret[ifaces_count - 1])  0)
+goto cleanup;
+
+if (virHashAddEntry(ifaces_store, ifname_s,
+ifaces_ret[ifaces_count - 1])  0)
+goto cleanup;
+
+iface = ifaces_ret[ifaces_count - 1];
+iface-naddrs = 0;
+
+if (VIR_STRDUP(iface-name, ifname_s)  0)
+goto error;
+
+/* hwaddr might be omitted */
+hwaddr = virJSONValueObjectGetString(tmp_iface, 
hardware-address);
+if (hwaddr  VIR_STRDUP(iface-hwaddr, hwaddr)  0)
+goto error;
+}
+
+/* Has to be freed for each interface. */
+

Re: [libvirt] [PATCHv3 2/6] virsh: Add vshCmdCompleter and vshOptCompleter

2013-09-01 Thread Tomas Meszaros
On 28/08/13 at 06:00am, Eric Blake wrote:
 [re-adding the list, which was accidentally omitted]
 
 On 08/28/2013 05:26 AM, Tomas Meszaros wrote:
  Per-option completions make sense.  For example, 'virsh vol-key --pool
  TAB' wants to use a pool completer, while 'virsh vol-key --vol TAB'
  wants to use a volume completer; furthermore, 'virsh vol-key TAB'
  should be the combination of the option completer (showing --vol and
  --pool) AND the volume completer filtered to names not starting with '-'
  (since virsh has the semantics that arguments are positional, where the
  option '--vol' is implied if the argument that appears in that position
  does not resemble an option).
  
  So If I get it right, you are suggesting that it should work like this:
  
  virsh # vol-key TAB
  vol1vol2pool1   pool2
  
  as you said, combination of --vol and --pool completers.
 
 No, it should work like this:
 
 virsh# vol-key TAB
 vol1 vol2 --vol --pool
 
 the combination of all (non-option) completions for the current
 available mandatory option (volume completer), and of all possible
 options that still make sense at this point in the command line.
 
 Likewise:
 
 virsh# vol-key vol TAB
 pool1 pool2 --pool
 
 virsh# vol-key -TAB
 --vol --pool
 
 virsh# vol-key vTAB
 vol1 vol2
 
 virsh# vol-key --pool TAB
 pool1 pool2
 
 virsh# vol-key --pool pool1 TAB
 vol1 vol2 --vol
 
 and so forth.

  
  
  I was initially thinking that completion should work like this:
  
  virsh # vol-key TAB
  vol1vol2
  
  It is completing vol first becasue vol is only mandatory argument
  for this command.
 
 It is a mandatory option, but mandatory options need not come first.
 Remember, our option parser allows mandatory options to occur
 positionally without an option name, OR to be interleaved in any other
 order by including the option string.
 
  
  
  Only if someone type:
  
  virsh # vol-key --pool TAB
  pool1   pool2
  
  then call --pool completer.
 
 This is correct - once an option is awaiting an argument, then the
 option completer must return nothing at that point in time.  But if you
 look at it as the union of two completers - the set of options that are
 still valid in the current context, and the set of strings that are
 valid assuming positional semantics of the current option, you will
 always get the right answer, without needing a per-command completer
 (just per-option completers).
 
 -- 
 Eric Blake   eblake redhat com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org
 
 
 

Yeah, It makes more sense.
I'm now rewriting the whole stuff to just use opt completers.

-- 
Tomas Meszaros

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


Re: [libvirt] [PATCH v2] kvm: warn if num cpus is greater than num recommended

2013-09-01 Thread Marcelo Tosatti
On Fri, Aug 23, 2013 at 03:24:37PM +0200, Andrew Jones wrote:
 The comment in kvm_max_vcpus() states that it's using the recommended
 procedure from the kernel API documentation to get the max number
 of vcpus that kvm supports. It is, but by always returning the
 maximum number supported. The maximum number should only be used
 for development purposes. qemu should check KVM_CAP_NR_VCPUS for
 the recommended number of vcpus. This patch adds a warning if a user
 specifies a number of cpus between the recommended and max.
 
 v2:
 Incorporate tests for max_cpus, which specifies the maximum number
 of hotpluggable cpus. An additional note is that the message for
 the fail case was slightly changed, 'exceeds max cpus' to
 'exceeds the maximum cpus'. If this is unacceptable change for
 users like libvirt, then I'll need to spin a v3.
 
 Signed-off-by: Andrew Jones drjo...@redhat.com
 ---
  kvm-all.c | 69 
 ---
  1 file changed, 40 insertions(+), 29 deletions(-)
 
 diff --git a/kvm-all.c b/kvm-all.c
 index a2d49786365e3..021f5f47e53da 100644
 --- a/kvm-all.c
 +++ b/kvm-all.c
 @@ -1322,24 +1322,20 @@ static int kvm_irqchip_create(KVMState *s)
  return 0;
  }
  
 -static int kvm_max_vcpus(KVMState *s)
 +/* Find number of supported CPUs using the recommended
 + * procedure from the kernel API documentation to cope with
 + * older kernels that may be missing capabilities.
 + */
 +static int kvm_recommended_vcpus(KVMState *s)
  {
 -int ret;
 -
 -/* Find number of supported CPUs using the recommended
 - * procedure from the kernel API documentation to cope with
 - * older kernels that may be missing capabilities.
 - */
 -ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
 -if (ret) {
 -return ret;
 -}
 -ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
 -if (ret) {
 -return ret;
 -}
 +int ret = kvm_check_extension(s, KVM_CAP_NR_VCPUS);
 +return (ret) ? ret : 4;
 +}
  
 -return 4;
 +static int kvm_max_vcpus(KVMState *s)
 +{
 +int ret = kvm_check_extension(s, KVM_CAP_MAX_VCPUS);
 +return (ret) ? ret : kvm_recommended_vcpus(s);
  }
  
  int kvm_init(void)
 @@ -1347,11 +1343,19 @@ int kvm_init(void)
  static const char upgrade_note[] =
  Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n
  (see http://sourceforge.net/projects/kvm).\n;
 +struct {
 +const char *name;
 +int num;
 +} num_cpus[] = {
 +{ SMP,  smp_cpus },
 +{ hotpluggable, max_cpus },
 +{ NULL, }
 +}, *nc = num_cpus;
 +int soft_vcpus_limit, hard_vcpus_limit;
  KVMState *s;
  const KVMCapabilityInfo *missing_cap;
  int ret;
  int i;
 -int max_vcpus;
  
  s = g_malloc0(sizeof(KVMState));
  
 @@ -1392,19 +1396,26 @@ int kvm_init(void)
  goto err;
  }
  
 -max_vcpus = kvm_max_vcpus(s);
 -if (smp_cpus  max_vcpus) {
 -ret = -EINVAL;
 -fprintf(stderr, Number of SMP cpus requested (%d) exceeds max cpus 
 -supported by KVM (%d)\n, smp_cpus, max_vcpus);
 -goto err;
 -}
 +/* check the vcpu limits */
 +soft_vcpus_limit = kvm_recommended_vcpus(s);
 +hard_vcpus_limit = kvm_max_vcpus(s);
  
 -if (max_cpus  max_vcpus) {
 -ret = -EINVAL;
 -fprintf(stderr, Number of hotpluggable cpus requested (%d) exceeds 
 max cpus 
 -supported by KVM (%d)\n, max_cpus, max_vcpus);
 -goto err;
 +while (nc-name) {
 +if (nc-num  soft_vcpus_limit) {
 +fprintf(stderr,
 +Warning: Number of %s cpus requested (%d) exceeds 
 +the recommended cpus supported by KVM (%d)\n,
 +nc-name, nc-num, soft_vcpus_limit);
 +
 +if (nc-num  hard_vcpus_limit) {
 +ret = -EINVAL;
 +fprintf(stderr, Number of %s cpus requested (%d) exceeds 
 +the maximum cpus supported by KVM (%d)\n,
 +nc-name, nc-num, hard_vcpus_limit);
 +goto err;
 +}
 +}
 +nc++;
  }
  
  s-vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
 -- 
 1.8.1.4

ACK

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


Re: [libvirt] [PATCH] qemu_hotplug: Resolve DEADCODE coverity error

2013-09-01 Thread John Ferlan
On 08/31/2013 07:24 AM, John Ferlan wrote:
 Remove unused 'cgroup' variable in qemuDomainAttachDeviceDiskLive() to
 resolve coverity DEADCODE complaint
 
 ---
 Refactoring of qemuDomainAttachDeviceDiskLive() in the following patch:
 
 https://www.redhat.com/archives/libvir-list/2013-August/msg00079.html
 
 seemed to wake up Coverity and it complained about deadcode as a result
 of 'cgroup' only ever been NULL thus the check for it to be non-NULL in
 order to call qemuTeardownDiskCgroup() would not be able to occur. The
 code actually hadn't changed, but for some reason coverity now found it.
 
 Followed example in qemuDomainChangeDiskMediaLive() and just removed the
 'cgroup' variable.
 
  src/qemu/qemu_hotplug.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)
 
Pushed.

John

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


Re: [libvirt] Can I request a new release of libvirt-java?

2013-09-01 Thread Daniel Veillard
On Fri, Aug 30, 2013 at 12:31:55PM +0200, Wido den Hollander wrote:
 On 08/15/2013 11:00 AM, Wido den Hollander wrote:
 Hello,
 
 In the recent months various new methods were added to libvirt-java
 which we (Apache CloudStack) would like to use in our KVM code.
 
 For example resizing storage volumes, right now we have to do this with
 Bash scripting since although libvirt supports resizing volumes, the
 current release (0.4.9) of libvirt-java doesn't.
 
 I don't know if there are any objections, but if possible I'd like to
 see 0.5.0 released so we get this new functionality for CloudStack.
 
 We use maven for building CloudStack and it fetches libvirt-java from
 libvirt.org/maven2
 
 
 Can I give this one a small bump?

  Oops, okay, point taken, not sure i can do this today, but I will
try this week !

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] Entering freeze for libvirt-1.1.2

2013-09-01 Thread Daniel Veillard
On Fri, Aug 30, 2013 at 02:58:35PM +0200, Viktor Mihajlovski wrote:
 On 08/28/2013 06:25 PM, Daniel Veillard wrote:
I am a day late but I finally tagged the release candidate 1
 of 1.1.2 in git and push the tarball and rpms to the usual place:
 
ftp://libvirt.org/libvirt/
 
   so the plan is to have an rc2 candidate on friday and if everything
 looks good push the final release 1.1.2 on Monday.
 
Please give it a try especially for potential portability issues,
 
 thanks in advance !
 
 Daniel
 
 Briefly kicked the tires on s390, no issues seen so far.

  Okay, thanks :-)

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


[libvirt] Release of libvirt-1.1.2

2013-09-01 Thread Daniel Veillard
 As scheduled I have pushed the tarballs and rpms of the new release at
the usual place:

  ftp://libvirt.org/libvirt/

 This is a medium sized release with less than 300 commits, with a
inclination toward code improvements and bugs fixes, in particular
fixes for 4 CVEs. It may be a good idea to upgrade !

Features:
- various improvements to libxl driver (Jim Fehlig, Bamvor Jian Zhang)
- systemd integration improvements (Daniel P. Berrange, Mooli Tayer)
- Add flag to BaselineCPU API to return detailed CPU features (Don Dugger)
- Introduce a virt-login-shell binary (Dan Walsh)
- conf: add startupPolicy attribute for harddisk (Guannan Ren)

Security:
- provide supplemental groups even when parsing label (CVE-2013-4291) (Eric 
Blake)
- Add bounds checking on virDomainMigrate*Params RPC calls (CVE-2013-4292) 
(Daniel P. Berrange)
- CVE-2013-5651 virbitmap: Refactor virBitmapParse to avoid access beyond 
bounds of array (Peter Krempa)
- CVE-2013-4239 xen: fix memory corruption in legacy driver (Jim Fehlig)

Documentation:
- Reformat disk attribute description in formatdomain (John Ferlan)
- Update iSCSI storage pool example (John Ferlan)
- Update formatsecrets to include more examples of each type (John Ferlan)
- Update the formatdomain disk examples (John Ferlan)
- Clean 09adfdc62de2b up (Michal Privoznik)
- virt-pki-validate: add --help/--version option (Eric Blake)
- virt-xml-validate: add --help/--version option (Eric Blake)
- Discourage users to set hard_limit (Michal Privoznik)
- Update polkit examples to use 'lookup' method (Daniel P. Berrange)
- fix usb node device sub-element names (Xuesong Zhang)
- virt-login-shell: improve error message grammar (Ruben Kerkhof)
- storage pool permission copy-paste fix (Philipp Hahn)
- mention VIR_TEST_RANGE (Eric Blake)
- Document use of systemd socket activation (Daniel P. Berrange)
- Remove leftovers from hyperv spinlocks documentation (Ján Tomko)
- Fix typo in domain name in polkit acl example (Daniel P. Berrange)
- Add documentation for access control system (Daniel P. Berrange)
- Add an example config file for virtlockd (Daniel P. Berrange)
- Add a man page for virtlockd daemon (Daniel P. Berrange)
- Add info about access control checks into API reference (Daniel P. Berrange)
- Fix minor typos in messages and docs (Yuri Chornoivan)

Portability:
- build: fix virtlockd file distribution (Eric Blake)
- build: shipped files must not depend on BUILT_SOURCES (Eric Blake)
- build: only create virt-login-shell for lxc builds (Eric Blake)
- qemu: Only setup vhost if virtType == kvm (Cole Robinson)
- Process virtlockd.conf instead of libvirtd.conf (Guido Günther)
- Change way we fake dbus method calls (Daniel P. Berrange)
- random: don't mix RAND_MAX with random_r (Eric Blake)
- tests: skip schema validation tests if xmllint is missing (Eric Blake)
- Check for --no-copy-dt-needed linker flag (Guido Günther)
- Simplify RELRO_LDFLAGS (Guido Günther)
- tests: fix building without xattr support (Claudio Bley)
- nwfilter: Don't fail to start if DBus isn't available (Peter Krempa)
- virsystemd: Don't fail to start VM if DBus isn't available or compiled in 
(Peter Krempa)
- tools: Make sure to distribute conf_DATA, fix RPM build (Cole Robinson)
- Directly link against needed libraries (Guido Günther)
- Directly link against needed libraries (Guido Günther)
- build: avoid -lgcrypt with newer gnutls (Eric Blake)
- build: more workarounds for if_bridge.h (Eric Blake)
- tests: avoid too-large constants (Eric Blake)
- tests: work with older dbus (Eric Blake)
- build: fix compilation of virt-login-shell.c (Jim Fehlig)
- maint: the compiler is not always named gcc (Eric Blake)
- build: fix qemuagenttest build with -O0 in fedora 19. (Jincheng Miao)
- spec: RHEL-7 does not have sanlock on i686 (Jiri Denemark)
- spec: Disable libssh2 support for RHEL (Peter Krempa)

Bug Fixes:
- qemu_hotplug: Resolve DEADCODE coverity error (John Ferlan)
- Fix memory leak in cmdAttachDisk (Hongwei Bi)
- python: Fix a PyList usage mistake (Guan Qiang)
- qemu: Remove hostdev entry when freeing the depending network entry (Peter 
Krempa)
- virsh: detect programming errors with option parsing (Eric Blake)
- virt-sanlock-cleanup; Fix augtool usage (Jiri Denemark)
- virsh: Fix debugging (Martin Kletzander)
- virsh: free the caps list properly if one of them is invalid (Ján Tomko)
- virsh: free the formatting string when listing pool details (Ján Tomko)
- virsh: free the list from ListAll APIs even for 0 items (Ján Tomko)
- virsh: free messages after logging them to a file (Ján Tomko)
- Test network update XML parsing (Ján Tomko)
- Always specify qcow2 compat level on qemu-img command line (Ján Tomko)
- virsh: fix return value error of cpu-stats (Guannan Ren)
- Don't free NULL network in cmdNetworkUpdate (Ján Tomko)
- schema: Allow dots in device aliases (Jiri Denemark)
- qemu: Don't update count of vCPUs if hot-plug fails silently (Peter Krempa)
- tests: Add URI precedence checking (Martin Kletzander)
- Fix 

Re: [libvirt] [PATCH 1/3] CPU: Implement guestData for PPC CPU driver

2013-09-01 Thread Li Zhang


Any comments about my CPU patches?


On 2013年08月29日 16:46, Li Zhang wrote:

From: Li Zhang zhlci...@linux.vnet.ibm.com

On Power platform, Power7+ can support Power7 guest.
It needs to define XML configuration to specify guest's CPU model.

For exmaple:
   cpu match='exact'
 modelPOWER7+_v2.1/model
 vendorIBM/vendor
   /cpu

Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
  src/cpu/cpu_powerpc.c | 166 +-
  1 file changed, 164 insertions(+), 2 deletions(-)

diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
index 647a8a1..84fa3f7 100644
--- a/src/cpu/cpu_powerpc.c
+++ b/src/cpu/cpu_powerpc.c
@@ -99,6 +99,23 @@ ppcModelFindPVR(const struct ppc_map *map,
  return NULL;
  }
  
+static struct ppc_model *

+ppcModelCopy(const struct ppc_model *model)
+{
+struct ppc_model *copy;
+
+if (VIR_ALLOC(copy)  0 ||
+VIR_STRDUP(copy-name, model-name)  0) {
+ppcModelFree(copy);
+return NULL;
+}
+
+copy-data.pvr = model-data.pvr;
+copy-vendor = model-vendor;
+
+return copy;
+}
+
  static struct ppc_vendor *
  ppcVendorFind(const struct ppc_map *map,
const char *name)
@@ -126,6 +143,29 @@ ppcVendorFree(struct ppc_vendor *vendor)
  VIR_FREE(vendor);
  }
  
+static struct ppc_model *

+ppcModelFromCPU(const virCPUDefPtr cpu,
+const struct ppc_map *map)
+{
+struct ppc_model *model = NULL;
+
+if ((model = ppcModelFind(map, cpu-model)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Unknown CPU model %s), cpu-model);
+goto error;
+}
+
+if ((model = ppcModelCopy(model)) == NULL)
+goto error;
+
+return model;
+
+error:
+ppcModelFree(model);
+return NULL;
+}
+
+
  static int
  ppcVendorLoad(xmlXPathContextPtr ctxt,
struct ppc_map *map)
@@ -288,6 +328,112 @@ error:
  return NULL;
  }
  
+static virCPUDataPtr

+ppcMakeCPUData(virArch arch, struct cpuPPCData *data)
+{
+virCPUDataPtr cpuData;
+
+if (VIR_ALLOC(cpuData)  0)
+return NULL;
+
+cpuData-arch = arch;
+cpuData-data.ppc = *data;
+data = NULL;
+
+return cpuData;
+}
+
+static virCPUCompareResult
+ppcCompute(virCPUDefPtr host,
+ const virCPUDefPtr cpu,
+ virCPUDataPtr *guestData,
+ char **message)
+
+{
+struct ppc_map *map = NULL;
+struct ppc_model *host_model = NULL;
+struct ppc_model *guest_model = NULL;
+
+int ret = 0;
+virArch arch;
+size_t i;
+
+if (cpu-arch != VIR_ARCH_NONE) {
+bool found = false;
+
+for (i = 0; i  ARRAY_CARDINALITY(archs); i++) {
+if (archs[i] == cpu-arch) {
+found = true;
+break;
+}
+}
+
+if (!found) {
+VIR_DEBUG(CPU arch %s does not match host arch,
+  virArchToString(cpu-arch));
+if (message 
+virAsprintf(message,
+_(CPU arch %s does not match host arch),
+virArchToString(cpu-arch))  0)
+goto error;
+return VIR_CPU_COMPARE_INCOMPATIBLE;
+}
+arch = cpu-arch;
+} else {
+arch = host-arch;
+}
+
+   if (cpu-vendor 
+(!host-vendor || STRNEQ(cpu-vendor, host-vendor))) {
+VIR_DEBUG(host CPU vendor does not match required CPU vendor %s,
+  cpu-vendor);
+if (message 
+virAsprintf(message,
+_(host CPU vendor does not match required 
+  CPU vendor %s),
+cpu-vendor)  0)
+goto error;
+return VIR_CPU_COMPARE_INCOMPATIBLE;
+}
+
+if (!(map = ppcLoadMap()) ||
+!(host_model = ppcModelFromCPU(host, map)) ||
+!(guest_model = ppcModelFromCPU(cpu, map)))
+goto error;
+
+if (guestData != NULL) {
+if (cpu-type == VIR_CPU_TYPE_GUEST 
+cpu-match == VIR_CPU_MATCH_STRICT 
+STRNEQ(guest_model-name, host_model-name)) {
+VIR_DEBUG(host CPU model does not match required CPU model %s,
+ guest_model-name);
+if (message 
+virAsprintf(message,
+_(host CPU model does not match required 
+CPU model %s),
+guest_model-name)  0)
+goto error;
+return VIR_CPU_COMPARE_INCOMPATIBLE;
+}
+
+if (!(*guestData = ppcMakeCPUData(arch, guest_model-data)))
+goto error;
+}
+
+ret = VIR_CPU_COMPARE_IDENTICAL;
+
+out:
+   ppcMapFree(map);
+   ppcModelFree(host_model);
+   ppcModelFree(guest_model);
+   return ret;
+
+error:
+   ret = VIR_CPU_COMPARE_ERROR;
+   goto out;
+
+}
+
  static virCPUCompareResult
  ppcCompare(virCPUDefPtr host,
 virCPUDefPtr cpu)
@@ -369,6 +515,15 @@ 

Re: [libvirt] [PATCH 1/3] CPU: Implement guestData for PPC CPU driver

2013-09-01 Thread Doug Goldstein
On Thu, Aug 29, 2013 at 3:46 AM, Li Zhang zhlci...@gmail.com wrote:

 From: Li Zhang zhlci...@linux.vnet.ibm.com

 On Power platform, Power7+ can support Power7 guest.
 It needs to define XML configuration to specify guest's CPU model.

 For exmaple:
   cpu match='exact'
 modelPOWER7+_v2.1/model
 vendorIBM/vendor
   /cpu

 Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
 ---
  src/cpu/cpu_powerpc.c | 166
 +-
  1 file changed, 164 insertions(+), 2 deletions(-)

 diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
 index 647a8a1..84fa3f7 100644
 --- a/src/cpu/cpu_powerpc.c
 +++ b/src/cpu/cpu_powerpc.c
 @@ -99,6 +99,23 @@ ppcModelFindPVR(const struct ppc_map *map,
  return NULL;
  }

 +static struct ppc_model *
 +ppcModelCopy(const struct ppc_model *model)
 +{
 +struct ppc_model *copy;
 +
 +if (VIR_ALLOC(copy)  0 ||
 +VIR_STRDUP(copy-name, model-name)  0) {
 +ppcModelFree(copy);
 +return NULL;
 +}
 +
 +copy-data.pvr = model-data.pvr;
 +copy-vendor = model-vendor;
 +
 +return copy;
 +}
 +
  static struct ppc_vendor *
  ppcVendorFind(const struct ppc_map *map,
const char *name)
 @@ -126,6 +143,29 @@ ppcVendorFree(struct ppc_vendor *vendor)
  VIR_FREE(vendor);
  }

 +static struct ppc_model *
 +ppcModelFromCPU(const virCPUDefPtr cpu,
 +const struct ppc_map *map)
 +{
 +struct ppc_model *model = NULL;
 +
 +if ((model = ppcModelFind(map, cpu-model)) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Unknown CPU model %s), cpu-model);
 +goto error;
 +}
 +
 +if ((model = ppcModelCopy(model)) == NULL)
 +goto error;
 +
 +return model;
 +
 +error:
 +ppcModelFree(model);
 +return NULL;
 +}
 +
 +
  static int
  ppcVendorLoad(xmlXPathContextPtr ctxt,
struct ppc_map *map)
 @@ -288,6 +328,112 @@ error:
  return NULL;
  }

 +static virCPUDataPtr
 +ppcMakeCPUData(virArch arch, struct cpuPPCData *data)
 +{
 +virCPUDataPtr cpuData;
 +
 +if (VIR_ALLOC(cpuData)  0)
 +return NULL;
 +
 +cpuData-arch = arch;
 +cpuData-data.ppc = *data;
 +data = NULL;
 +
 +return cpuData;
 +}
 +
 +static virCPUCompareResult
 +ppcCompute(virCPUDefPtr host,
 + const virCPUDefPtr cpu,
 + virCPUDataPtr *guestData,
 + char **message)
 +
 +{
 +struct ppc_map *map = NULL;
 +struct ppc_model *host_model = NULL;
 +struct ppc_model *guest_model = NULL;
 +
 +int ret = 0;
 +virArch arch;
 +size_t i;
 +
 +if (cpu-arch != VIR_ARCH_NONE) {
 +bool found = false;
 +
 +for (i = 0; i  ARRAY_CARDINALITY(archs); i++) {
 +if (archs[i] == cpu-arch) {
 +found = true;
 +break;
 +}
 +}
 +
 +if (!found) {
 +VIR_DEBUG(CPU arch %s does not match host arch,
 +  virArchToString(cpu-arch));
 +if (message 
 +virAsprintf(message,
 +_(CPU arch %s does not match host arch),
 +virArchToString(cpu-arch))  0)
 +goto error;
 +return VIR_CPU_COMPARE_INCOMPATIBLE;
 +}
 +arch = cpu-arch;
 +} else {
 +arch = host-arch;
 +}
 +
 +   if (cpu-vendor 
 +(!host-vendor || STRNEQ(cpu-vendor, host-vendor))) {
 +VIR_DEBUG(host CPU vendor does not match required CPU vendor %s,
 +  cpu-vendor);
 +if (message 
 +virAsprintf(message,
 +_(host CPU vendor does not match required 
 +  CPU vendor %s),
 +cpu-vendor)  0)
 +goto error;
 +return VIR_CPU_COMPARE_INCOMPATIBLE;
 +}


Could this beginning part of ppcCompute() not go into some common function
in cpu_generic.c? It just looks entirely copied and pasted from cpu_x86.c
from x86Compute()


 +
 +if (!(map = ppcLoadMap()) ||
 +!(host_model = ppcModelFromCPU(host, map)) ||
 +!(guest_model = ppcModelFromCPU(cpu, map)))
 +goto error;
 +
 +if (guestData != NULL) {
 +if (cpu-type == VIR_CPU_TYPE_GUEST 
 +cpu-match == VIR_CPU_MATCH_STRICT 
 +STRNEQ(guest_model-name, host_model-name)) {
 +VIR_DEBUG(host CPU model does not match required CPU model
 %s,
 + guest_model-name);
 +if (message 
 +virAsprintf(message,
 +_(host CPU model does not match required 
 +CPU model %s),
 +guest_model-name)  0)
 +goto error;
 +return VIR_CPU_COMPARE_INCOMPATIBLE;
 +}
 +
 +if (!(*guestData = ppcMakeCPUData(arch, guest_model-data)))
 +goto error;
 +}
 +
 +ret = 

Re: [libvirt] [PATCHv3 1/4] VMX: Create virVMXFormatDisk() from HD and CD-ROM

2013-09-01 Thread Doug Goldstein
On Thu, Aug 29, 2013 at 5:19 AM, Michal Privoznik mpriv...@redhat.comwrote:

 On 28.08.2013 23:53, Doug Goldstein wrote:
  virVMXFormatHardDisk() and virVMXFormatCDROM() duplicated a lot of code
  from each other and made a lot of nested if checks to build each part of
  the VMX file. This hopefully simplifies the code path while combining
  the two functions with no net difference.
  ---
   src/libvirt_vmx.syms |   3 +-
   src/vmx/vmx.c| 192
 +--
   src/vmx/vmx.h|   5 +-
   3 files changed, 64 insertions(+), 136 deletions(-)
 
  diff --git a/src/libvirt_vmx.syms b/src/libvirt_vmx.syms
  index 206ad44..e673923 100644
  --- a/src/libvirt_vmx.syms
  +++ b/src/libvirt_vmx.syms
  @@ -6,11 +6,10 @@
   virVMXConvertToUTF8;
   virVMXDomainXMLConfInit;
   virVMXEscapeHex;
  -virVMXFormatCDROM;
   virVMXFormatConfig;
  +virVMXFormatDisk;
   virVMXFormatEthernet;
   virVMXFormatFloppy;
  -virVMXFormatHardDisk;
   virVMXFormatParallel;
   virVMXFormatSerial;
   virVMXFormatVNC;
  diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
  index 35afe26..f5cb9fe 100644
  --- a/src/vmx/vmx.c
  +++ b/src/vmx/vmx.c
  @@ -517,7 +517,6 @@ VIR_ENUM_IMPL(virVMXControllerModelSCSI,
 VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
 UNUSED lsisas1078);
 
 
  -
   /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
 * * * *
* Helpers
*/
  @@ -3213,14 +3212,8 @@ virVMXFormatConfig(virVMXContext *ctx,
 virDomainXMLOptionPtr xmlopt, virDomainDe
   for (i = 0; i  def-ndisks; ++i) {
   switch (def-disks[i]-device) {
 case VIR_DOMAIN_DISK_DEVICE_DISK:
  -if (virVMXFormatHardDisk(ctx, def-disks[i], buffer)  0) {
  -goto cleanup;
  -}
  -
  -break;
  -
 case VIR_DOMAIN_DISK_DEVICE_CDROM:
  -if (virVMXFormatCDROM(ctx, def-disks[i], buffer)  0) {
  +if (virVMXFormatDisk(ctx, def-disks[i], buffer)  0) {
   goto cleanup;
   }
 
  @@ -3369,67 +3362,89 @@ virVMXFormatVNC(virDomainGraphicsDefPtr def,
 virBufferPtr buffer)
   return 0;
   }
 
  -
  -
   int
  -virVMXFormatHardDisk(virVMXContext *ctx, virDomainDiskDefPtr def,
  +virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def,
virBufferPtr buffer)
   {
   int controllerOrBus, unit;
  -const char *busName = NULL;
  -const char *entryPrefix = NULL;
  -const char *deviceTypePrefix = NULL;
  +const char *vmxDeviceType = NULL;
   char *fileName = NULL;
 
  -if (def-device != VIR_DOMAIN_DISK_DEVICE_DISK) {
  -virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Invalid
 argument));
  +/* Convert a handful of types to their string values */
  +const char *busType = virDomainDiskBusTypeToString(def-bus);
  +const char *deviceType = virDomainDeviceTypeToString(def-device);
  +const char *diskType = virDomainDeviceTypeToString(def-type);
  +
  +/* If we are dealing with a disk its a .vmdk, otherwise it must be
  + * an ISO.
  + */
  +const char *fileExt = (def-device == VIR_DOMAIN_DISK_DEVICE_DISK) ?
  +.vmdk : .iso;
  +
  +/* Check that we got a valid device type */
  +if (def-device != VIR_DOMAIN_DISK_DEVICE_DISK 
  +def-device != VIR_DOMAIN_DISK_DEVICE_CDROM) {

 Indentation looks off.

  +virReportError(VIR_ERR_INTERNAL_ERROR,
  +   _(Invalid device type supplied: %s),
 deviceType);
   return -1;
   }
 
  -if (def-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
  -busName = SCSI;
  -entryPrefix = scsi;
  -deviceTypePrefix = scsi;
  +/* We only support type='file' and type='block' */
  +if (def-type != VIR_DOMAIN_DISK_TYPE_FILE 
  +def-type != VIR_DOMAIN_DISK_TYPE_BLOCK) {

 And again.

  +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
  +   _(%s %s '%s' has unsupported type '%s',
 expecting 
  +   '%s' or '%s'), busType, deviceType, def-dst,
 diskType,

 And again.

  +
 virDomainDiskTypeToString(VIR_DOMAIN_DISK_TYPE_FILE),
  +
 virDomainDiskTypeToString(VIR_DOMAIN_DISK_TYPE_BLOCK));
  +return -1;
  +}
 
  +if (def-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
   if (virVMXSCSIDiskNameToControllerAndUnit(def-dst,
 controllerOrBus,
 unit)  0) {
   return -1;
   }
   } else if (def-bus == VIR_DOMAIN_DISK_BUS_IDE) {
  -busName = IDE;
  -entryPrefix = ide;
  -deviceTypePrefix = ata;
  -
   if (virVMXIDEDiskNameToBusAndUnit(def-dst, controllerOrBus,
  -   unit)  0) {
  +  unit)  0) {
   return -1;
   }
   } else {
   virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
  -