Re: [libvirt] [PATCH 3/5] net: use virDomainNICModelType{From|To}String functions
On 01/14/2013 10:15 PM, John Ferlan wrote:
On 01/13/2013 10:34 AM, Guannan Ren wrote:
if (def-model) {
virBufferEscapeString(buf, model type='%s'/\n,
- def-model);
-if (STREQ(def-model, virtio)
+ virDomainNICModelTypeToString(def-model));
+if ((def-model == VIR_DOMAIN_NIC_MODEL_VIRTIO)
(def-driver.virtio.name || def-driver.virtio.txmode)) {
virBufferAddLit(buf, driver);
if (def-driver.virtio.name) {
Since model can be VIR_DOMAIN_NIC_MODEL_DEFAULT (zero), is this what
you really want?
if (def-model)
virBufferEscapeString(buf, model type='%s'/\n,
virDomainNICModelTypeToString(def-model));
if def-model is VIR_DOMAIN_NIC_MODEL_DEFAULT(0),
virDomainNICModelTypeToString
will not be executed.
For input XML VIR_DOMAIN_NIC_MODEL_DEFAULT means no particular
model is specified.
in hypervisors code, if often to set it to a default value, for
qemu , it is rtl8139.
For output XML almost, if def-mode is still
VIR_DOMAIN_NIC_MODEL_DEFAULT, it will skip
printing the model attribute. But there is slightly different
between each of hypervisors.
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index c604bd2..0409b0b 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2597,10 +2597,10 @@ virVMXParseEthernet(virConfPtr conf, int controller,
virDomainNetDefPtr *def)
/* Setup virDomainNetDef */
if (connectionType == NULL || STRCASEEQ(connectionType, bridged)) {
(*def)-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-(*def)-model = virtualDev;
+(*def)-model = virDomainNICModelTypeFromString(virtualDev);
What if virDomainNICModelTypeFromString() 0
virtualDev is guarantee to be a valid NIC model string in the codes
above, so
there is no possibility for it to return -1.
Guannan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] net: use virDomainNICModelType{From|To}String functions
On 01/14/2013 10:25 PM, John Ferlan wrote:
oops - one more ...
On 01/13/2013 10:34 AM, Guannan Ren wrote:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 661cc0f..a5ce119 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -878,7 +878,7 @@ struct _virDomainActualNetDef {
struct _virDomainNetDef {
enum virDomainNetType type;
virMacAddr mac;
-char *model;
+int model;
Should this be?
enum virDomainNICModel model;
for consistency.
Some places where we write code like this:
if ((net-model = virDomainNICModelTypeFromString(model)) 0)
...
If the model is type of enum and compile it with gcc option
-Werror=type-limits
gcc will report like:
error: comparison of unsigned expression 0 is always false
[-Werror=type-limits]
so we still need int type here.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt: lxc: don't mkdir when selinux is disabled
On Tue, Jan 15, 2013 at 10:13:36AM +0800, Gao feng wrote:
On 2013/01/09 19:20, Gao feng wrote:
libvirt lxc will fail to start when selinux is disabled.
error: Failed to start domain noroot
error: internal error guest failed to start: PATH=/bin:/sbin TERM=linux
container=lxc-libvirt container_uuid=b9873916-3516-c199-8112-1592ff694a9e
LIBVIRT_LXC_UUID=b9873916-3516-c199-8112-1592ff694a9e
LIBVIRT_LXC_NAME=noroot /bin/sh
2013-01-09 11:04:05.384+: 1: info : libvirt version: 1.0.1
2013-01-09 11:04:05.384+: 1: error : lxcContainerMountBasicFS:546 :
Failed to mkdir /sys/fs/selinux: No such file or directory
2013-01-09 11:04:05.384+: 7536: info : libvirt version: 1.0.1
2013-01-09 11:04:05.384+: 7536: error : virLXCControllerRun:1466 :
error receiving signal from container: Input/output error
2013-01-09 11:04:05.404+: 7536: error : virCommandWait:2287 : internal
error Child process (ip link del veth1) unexpected exit status 1: Cannot
find device veth1
fix this problem by checking if selinuxfs is mounted
in host before we try to create dir /sys/fs/selinux.
Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
Ping...
src/lxc/lxc_container.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 9f22923..d7f4960 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -540,13 +540,6 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
VIR_DEBUG(Processing %s - %s,
mnts[i].src, mnts[i].dst);
-if (virFileMakePath(mnts[i].dst) 0) {
-virReportSystemError(errno,
- _(Failed to mkdir %s),
- mnts[i].src);
-goto cleanup;
-}
-
srcpath = mnts[i].src;
/* Skip if mount doesn't exist in source */
@@ -554,6 +547,13 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
(access(srcpath, R_OK) 0))
continue;
+if (virFileMakePath(mnts[i].dst) 0) {
+virReportSystemError(errno,
+ _(Failed to mkdir %s),
+ mnts[i].src);
+goto cleanup;
+}
+
VIR_DEBUG(Mount %s on %s type=%s flags=%x, opts=%s,
srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags,
mnts[i].opts);
if (mount(srcpath, mnts[i].dst, mnts[i].type, mnts[i].mflags,
mnts[i].opts) 0) {
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 9/9] Add 'lxc-enter-namespace' command to virsh
On Mon, Jan 14, 2013 at 05:42:18PM -0700, Eric Blake wrote:
On 12/21/2012 10:08 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Add a 'lxc-enter-namespace' command which accepts a domain name
and then a command + args to run, attached to the container
eg
virsh -c lxc:/// lxc-enter-namespace demo -- /bin/ps -auxf
+static const vshCmdOptDef opts_lxc_enter_namespace[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{timeout, VSH_OT_INT, VSH_OFLAG_REQ_OPT, N_(timeout seconds. must
be positive.)},
+{async, VSH_OT_BOOL, 0, N_(execute namespace without waiting for
timeout)},
+{block, VSH_OT_BOOL, 0, N_(execute namespace without timeout)},
Oh, and I don't see timeout, async, or block actually used in the
implementation; did you mean to wire them up? Waiting for v2.
-ETOOMUCHCOPYANDPASTE
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 v3 2/3] build command line for pci-bridge device of qemu
On Mon, Jan 14, 2013 at 05:23:54PM +0100, Ján Tomko wrote:
On 01/10/13 02:04, liguang wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 04a9512..48b4f46 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -966,13 +966,6 @@ static char
*qemuPCIAddressAsString(virDomainDeviceInfoPtr dev)
{
char *addr;
-if (dev-addr.pci.domain != 0 ||
-dev-addr.pci.bus != 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Only PCI domain 0 and bus 0 are available));
-return NULL;
-}
-
if (virAsprintf(addr, %d:%d:%d.%d,
dev-addr.pci.domain,
dev-addr.pci.bus,
@@ -984,8 +977,24 @@ static char
*qemuPCIAddressAsString(virDomainDeviceInfoPtr dev)
return addr;
}
+static int qemuPciBridgeSupport(virDomainDefPtr def)
+{
+int i, c = 0;
+
+for (i = 0; i def-ncontrollers; i++) {
+virDomainControllerDefPtr controller = def-controllers[i];
+
+if (controller-type == VIR_DOMAIN_CONTROLLER_TYPE_PCIBRIDGE)
+c++;
+}
-static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
+if (c 1)
+return 0;
+else
+return -1;
+}
+
+static int qemuCollectPCIAddress(virDomainDefPtr def,
virDomainDeviceDefPtr device,
virDomainDeviceInfoPtr info,
void *opaque)
@@ -1004,6 +1013,20 @@ static int qemuCollectPCIAddress(virDomainDefPtr def
ATTRIBUTE_UNUSED,
return 0;
}
+if (info-addr.pci.domain != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Only PCI device addresses with
+ domain=0 are supported));
+return -1;
+}
+
+if (info-addr.pci.bus != 0 qemuPciBridgeSupport(def) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Only PCI device addresses with bus=0 are
+ supported without pci-bridge support));
+return -1;
+}
+
This would allow any bus number, even if we don't have enough PCI
bridges. It is also not the only place where qemuPCIAddressAsString is
called from and where this needs to be checked.
With other types of addresses, we will auto-create the controller
elements if we find the controller does not exist. We should probably
do the same for PCI. ie if you have bus == 2 and no PCI bridge for
that bus exists, then add one.
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 6/9 v2] nodedev: Dump max vports and vports in use for HBA's XML
This enrichs HBA's xml by dumping the number of max vports and
vports in use. Format is like:
capability type='vport_ops'
max_vports164/max_vports
vports5/vports
/capability
* docs/formatnode.html.in: (Document the new XML)
* docs/schemas/nodedev.rng: (Add the schema)
* src/conf/node_device_conf.h: (New member for data.scsi_host)
* src/node_device/node_device_linux_sysfs.c: (Collect the value of
max_vports and vports)
---
docs/formatnode.html.in | 10 --
docs/schemas/nodedev.rng |6 +++
src/conf/node_device_conf.c |7 +++-
src/conf/node_device_conf.h |2 +
src/node_device/node_device_linux_sysfs.c | 49 ++--
5 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index fcf..5712bcf 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -136,9 +136,13 @@
ddThe SCSI host number./dd
dtcodecapability/code/dt
ddCurrent capabilities include vports_ops (indicates
-vport operations are supported) and fc_host, the later
-implies following sub-elements: codewwnn/code,
-codewwpn/code, codefabric_wwn/code.
+vport operations are supported) and fc_host. vport_ops
+could contain two optional sub-elements: codevports/code,
+and codemax_vports/code. codevports/code shows the
+number of vport in use. codemax_vports/code shows the
+maximum vports the HBA supports. fc_host implies following
+sub-elements: codewwnn/code, codewwpn/code, and
+codefabric_wwn/code.
/dd
/dl
/dd
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 7c85815..b94cce6 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -267,6 +267,12 @@
attribute name='type'
valuevports_ops/value
/attribute
+element name='max_vports'
+ ref name='unsignedInt'/
+/element
+element name='vports'
+ ref name='unsignedInt'/
+/element
/define
define name='capscsihost'
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 819e6af..5962d58 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -392,7 +392,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def)
virBufferAddLit(buf, /capability\n);
}
if (data-scsi_host.flags VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) {
-virBufferAddLit(buf, capability type='vport_ops' /\n);
+virBufferAddLit(buf, capability type='vport_ops'\n);
+virBufferAsprintf(buf, max_vports%d/max_vports\n,
+ data-scsi_host.max_vports);
+virBufferAsprintf(buf, vports%d/vports\n,
+ data-scsi_host.vports);
+virBufferAddLit(buf, /capability\n);
}
break;
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 145d699..4e584a3 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -137,6 +137,8 @@ struct _virNodeDevCapsDef {
char *wwpn;
char *fabric_wwn;
unsigned int flags;
+int max_vports;
+int vports;
} scsi_host;
struct {
char *name;
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index 85bbab6..a1c3637 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -40,6 +40,8 @@
int
detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
{
+char *max_vports = NULL;
+char *vports = NULL;
int ret = -1;
VIR_DEBUG(Checking if host%d is an FC HBA, d-scsi_host.host);
@@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
d-scsi_host.host,
port_name,
- d-scsi_host.wwpn) == -1) {
+ d-scsi_host.wwpn) 0) {
VIR_ERROR(_(Failed to read WWPN for host%d), d-scsi_host.host);
goto cleanup;
}
@@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
d-scsi_host.host,
node_name,
- d-scsi_host.wwnn) == -1) {
+ d-scsi_host.wwnn) 0) {
VIR_ERROR(_(Failed to read WWNN for host%d), d-scsi_host.host);
goto cleanup;
}
@@ -66,23 +68,62 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
Re: [libvirt] [PATCH 7/9] nodedev: Fix the improper logic when enumerating SRIOV VF
On 2013年01月15日 03:42, Michal Privoznik wrote:
On 14.01.2013 15:34, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no virtfn
entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions) ||
data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with virtual_function cap.
However, this results in a VF will aslo get virtual_function cap.
This patch fixes it by:
* Ignoring the VF which has failure of getting PCI config space
(given that the successfully detected VFs are kept , it makes
sense to not give up on the failure of one VF too) with a warning,
so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */
int rc = pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions);
s/int rc/int ret/
if (ret 0 )
goto out;
else if (!ret (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
Hm.. what about the second condition being:
else if (data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= ...;
My rationale is - the first 'if' catches all the errors, so we don't
have to check 'ret' again for success. We already know it succeeded
because we are taking the 'else' branch. Having said that, what about
going one step further?
int ret = pciGetVirtualFunctions(...);
if (ret 0)
goto error;
if (data-pci_dev.num_virtual_functions)
data-pci_dev.flags |= ...;
Agreed. The function now only returns -1 or 0.
That is - we can leave the 'else' out since we are doing 'goto'.
Likewise, ' 0' can be left out because pciGetVirtualFunctions() sets
nonzero value there.
---
src/node_device/node_device_hal.c | 11 ---
src/node_device/node_device_udev.c | 11 ---
src/util/virpci.c | 36 +++-
3 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index 1afa21c..d0c419d 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -151,10 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char
*udi,
if (!pciGetPhysicalFunction(sysfs_path,d-pci_dev.physical_function))
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(sysfs_path,d-pci_dev.virtual_functions,
-d-pci_dev.num_virtual_functions) ||
-d-pci_dev.num_virtual_functions 0)
+int ret = pciGetVirtualFunctions(sysfs_path,
+d-pci_dev.virtual_functions,
+d-pci_dev.num_virtual_functions);
+if (ret 0) {
+VIR_FREE(sysfs_path);
+return -1;
+} else if (!ret (d-pci_dev.num_virtual_functions 0)) {
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
+}
VIR_FREE(sysfs_path);
}
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 87f1000..47ac4f4 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device,
union _virNodeDevCapData *data =def-caps-data;
int ret = -1;
char *p;
+int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device,
if (!pciGetPhysicalFunction(syspath,data-pci_dev.physical_function))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(syspath,data-pci_dev.virtual_functions,
-data-pci_dev.num_virtual_functions) ||
-data-pci_dev.num_virtual_functions 0)
+rc = pciGetVirtualFunctions(syspath,
+data-pci_dev.virtual_functions,
+data-pci_dev.num_virtual_functions);
+/* Out of memory */
+if (rc 0)
+goto out;
+else if (!rc (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0fb9923..9f4f3c7 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path,
unsigned int *num_virtual_functions)
{
int ret =
[libvirt] [PATCH 7/9 v2] nodedev: Fix the improper logic when enumerating SRIOV VF
pciGetVirtualFunctions returns 0 even if there is no virtfn
entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions) ||
data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with virtual_function cap.
However, this results in a VF will aslo get virtual_function cap.
This patch fixes it by:
* Ignoring the VF which has failure of getting PCI config space
(given that the successfully detected VFs are kept , it makes
sense to not give up on the failure of one VF too) with a warning,
so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */
int ret = pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions);
if (ret 0 )
goto out;
if (data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
---
src/node_device/node_device_hal.c | 12 +++--
src/node_device/node_device_udev.c | 11 ++--
src/util/virpci.c | 46 +---
3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index e64d6ac..6da5873 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -151,9 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char
*udi,
if (!pciGetPhysicalFunction(sysfs_path, d-pci_dev.physical_function))
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(sysfs_path, d-pci_dev.virtual_functions,
-d-pci_dev.num_virtual_functions) ||
-d-pci_dev.num_virtual_functions 0)
+int ret = pciGetVirtualFunctions(sysfs_path,
+ d-pci_dev.virtual_functions,
+ d-pci_dev.num_virtual_functions);
+if (ret 0) {
+VIR_FREE(sysfs_path);
+return -1;
+}
+
+if (d-pci_dev.num_virtual_functions 0)
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path);
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 62f6320..a90217d 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device,
union _virNodeDevCapData *data = def-caps-data;
int ret = -1;
char *p;
+int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device,
if (!pciGetPhysicalFunction(syspath, data-pci_dev.physical_function))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(syspath, data-pci_dev.virtual_functions,
-data-pci_dev.num_virtual_functions) ||
-data-pci_dev.num_virtual_functions 0)
+rc = pciGetVirtualFunctions(syspath,
+data-pci_dev.virtual_functions,
+data-pci_dev.num_virtual_functions);
+/* Out of memory */
+if (rc 0)
+goto out;
+else if (!rc (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0fb9923..ee4b3a8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path,
unsigned int *num_virtual_functions)
{
int ret = -1;
+int i;
DIR *dir = NULL;
struct dirent *entry = NULL;
char *device_link = NULL;
@@ -1989,45 +1990,52 @@ pciGetVirtualFunctions(const char *sysfs_path,
*num_virtual_functions = 0;
while ((entry = readdir(dir))) {
if (STRPREFIX(entry-d_name, virtfn)) {
+struct pci_config_address *config_addr = NULL;
if (virBuildPath(device_link, sysfs_path, entry-d_name) == -1) {
virReportOOMError();
-goto out;
+goto error;
}
VIR_DEBUG(Number of virtual functions: %d,
*num_virtual_functions);
-if (VIR_REALLOC_N(*virtual_functions,
-
Re: [libvirt] [PATCH 7/9 v2] nodedev: Fix the improper logic when enumerating SRIOV VF
On 2013年01月15日 17:56, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no virtfn
entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions) ||
data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with virtual_function cap.
However, this results in a VF will aslo get virtual_function cap.
This patch fixes it by:
* Ignoring the VF which has failure of getting PCI config space
(given that the successfully detected VFs are kept , it makes
sense to not give up on the failure of one VF too) with a warning,
so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */
int ret = pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions);
if (ret 0 )
goto out;
if (data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
---
src/node_device/node_device_hal.c | 12 +++--
src/node_device/node_device_udev.c | 11 ++--
src/util/virpci.c | 46 +---
3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index e64d6ac..6da5873 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -151,9 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char
*udi,
if (!pciGetPhysicalFunction(sysfs_path,d-pci_dev.physical_function))
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(sysfs_path,d-pci_dev.virtual_functions,
-d-pci_dev.num_virtual_functions) ||
-d-pci_dev.num_virtual_functions 0)
+int ret = pciGetVirtualFunctions(sysfs_path,
+d-pci_dev.virtual_functions,
+d-pci_dev.num_virtual_functions);
+if (ret 0) {
+VIR_FREE(sysfs_path);
+return -1;
+}
+
+if (d-pci_dev.num_virtual_functions 0)
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path);
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 62f6320..a90217d 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device,
union _virNodeDevCapData *data =def-caps-data;
int ret = -1;
char *p;
+int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device,
if (!pciGetPhysicalFunction(syspath,data-pci_dev.physical_function))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(syspath,data-pci_dev.virtual_functions,
-data-pci_dev.num_virtual_functions) ||
-data-pci_dev.num_virtual_functions 0)
+rc = pciGetVirtualFunctions(syspath,
+data-pci_dev.virtual_functions,
+data-pci_dev.num_virtual_functions);
+/* Out of memory */
+if (rc 0)
+goto out;
+else if (!rc (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0fb9923..ee4b3a8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path,
unsigned int *num_virtual_functions)
{
int ret = -1;
+int i;
DIR *dir = NULL;
struct dirent *entry = NULL;
char *device_link = NULL;
@@ -1989,45 +1990,52 @@ pciGetVirtualFunctions(const char *sysfs_path,
*num_virtual_functions = 0;
while ((entry = readdir(dir))) {
if (STRPREFIX(entry-d_name, virtfn)) {
+struct pci_config_address *config_addr = NULL;
if (virBuildPath(device_link, sysfs_path, entry-d_name) == -1) {
virReportOOMError();
-goto out;
+goto error;
}
VIR_DEBUG(Number of virtual functions: %d,
*num_virtual_functions);
-if (VIR_REALLOC_N(*virtual_functions,
-(*num_virtual_functions) + 1) != 0) {
-virReportOOMError();
-
[libvirt] the patch bridge: export multicast database via netlink broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use)
Eric Blake skrev 15.1.2013 01:57:
On 01/13/2013 01:05 PM, Thomas Backlund wrote:
Thomas Backlund skrev 13.1.2013 20:38:
patch both inline and attached as thunderbird tends to mess up ...
-
if_bridge.h uses struct in6_addr ip6; but does not include the in6.h
header.
Found by trying to build libvirt and connman against 3.8-rc3 headers.
Ok,
ignore this patch, it's not the correct fix as it introduces
redefinitions...
Btw, the error that I hit that made me suggest this fix was libvirt
config check bailing out:
config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has
incomplete type
Hmm, just now noticing this thread, after independently hitting and and
having to work around the same problem in libvirt:
https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html
https://bugzilla.redhat.com/show_bug.cgi?id=895141
Yep,
and the commit breaking uapi headers is by using struct in6_addr ip6 is:
From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001
From: Cong Wang amw...@redhat.com
Date: Fri, 7 Dec 2012 00:04:48 +
Subject: [PATCH] bridge: export multicast database via netlink
V5: fix two bugs pointed out by Thomas
remove seq check for now, mark it as TODO
V4: remove some useless #include
some coding style fix
V3: drop debugging printk's
update selinux perm table as well
V2: drop patch 1/2, export ifindex directly
Redesign netlink attributes
Improve netlink seq check
Handle IPv6 addr as well
This patch exports bridge multicast database via netlink
message type RTM_GETMDB. Similar to fdb, but currently bridge-specific.
We may need to support modify multicast database too (RTM_{ADD,DEL}MDB).
(Thanks to Thomas for patient reviews)
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: Stephen Hemminger shemmin...@vyatta.com
Cc: David S. Miller da...@davemloft.net
Cc: Thomas Graf tg...@suug.ch
Cc: Jesper Dangaard Brouer bro...@redhat.com
Signed-off-by: Cong Wang amw...@redhat.com
Acked-by: Thomas Graf tg...@suug.ch
Signed-off-by: David S. Miller da...@davemloft.net
---
include/uapi/linux/if_bridge.h | 55 ++
include/uapi/linux/rtnetlink.h |3 +
net/bridge/Makefile|2 +-
net/bridge/br_mdb.c| 163
net/bridge/br_multicast.c |1 +
net/bridge/br_private.h|1 +
security/selinux/nlmsgtab.c|1 +
7 files changed, 225 insertions(+), 1 deletions(-)
create mode 100644 net/bridge/br_mdb.c
--
Thomas
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC qom-cpu v2 1/2] target-i386: Convert CPU definitions into X86CPU subclasses
On Tue, Jan 15, 2013 at 09:41:04AM +0100, Igor Mammedov wrote: On Mon, 10 Dec 2012 23:59:31 +0100 Andreas Färber afaer...@suse.de wrote: TODO: sort classes for -cpu ?, generalize X86CPUListState, more testing Signed-off-by: Andreas Färber afaer...@suse.de Cc: Eduardo Habkost ehabk...@redhat.com Cc: Igor Mammedov imamm...@redhat.com [...] The patch is just renaming of the current builtin_x86_defs into a bunch of functions and polluting X86CPUClass with fields from the former x86_def_t. object_new() still creates a dummy cpu instance whose defaults are still manually copied from X86CPUClass instead of x86_def_t. That's a good thing, isn't it? It means the patch is easier to review. :-) No patch alone will do everything we want, because we want to do a lot. We need to do it one step at a time. (BTW, why are you looking at this RFC instead of the more recent one, that I have sent on Jan 4? [that's very similar to this one]) What's the point in having dummy sub-classes? How it can help in your CPU re-factoring? It will help us to unify the CPU creation/realization code that's duplicated over all the architectures. It will give libvirt an easy mechanism to list the available CPU models that won't require parsing help output (using qom-list-types QMP command). On the other hand converting features to static properties first and then converting X86CPU to sub-classes, yields already initialized to defaults sub-class completely removing notion of x86_def_t and not polluting X86CPUClass with redundant fields. I wouldn't disagree with this approach in principle, but I believe our main problem today is lack of reviewer bandwidth. I learned the hard way that trying to clean up everything before implementing something will make sure the code takes forever to be reviewed. For example see following patches on https://github.com/imammedo/qemu/commits/x86-cpu-classes.Jan142013 e9fd18f qdev: extend DEFINE_GENERIC_PROP() to support default values c65eca9 qdev: make qdev_prop_find_bit return non const so prop default value could be modified 0311952 allow to expolit default value static props for model, family, stepping vendor props 8b3080e target-i386: add helpers to change default values of static properties before object is created ed506d3 target-i386: prepare for subclasses to have its own instance of static properties definitions a48e252 target-i386: declare subclass for qemu64 cpu model 9c556c2 target-i386: move cpu_x86_init() cpu_x86_register() into it and switch to subclasses. PS: implemended only for qemu64 f5dbfe6 CPU_CLASS_NAME(qemu64) hack 00e15b8 target-i386: properties list are per subclass: do not set them in superclass to avoid defaults set by subclass be over-written -- Regards, Igor -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9 v2] nodedev: Dump max vports and vports in use for HBA's XML
On 15.01.2013 10:38, Osier Yang wrote:
This enrichs HBA's xml by dumping the number of max vports and
vports in use. Format is like:
capability type='vport_ops'
max_vports164/max_vports
vports5/vports
/capability
* docs/formatnode.html.in: (Document the new XML)
* docs/schemas/nodedev.rng: (Add the schema)
* src/conf/node_device_conf.h: (New member for data.scsi_host)
* src/node_device/node_device_linux_sysfs.c: (Collect the value of
max_vports and vports)
---
docs/formatnode.html.in | 10 --
docs/schemas/nodedev.rng |6 +++
src/conf/node_device_conf.c |7 +++-
src/conf/node_device_conf.h |2 +
src/node_device/node_device_linux_sysfs.c | 49 ++--
5 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index fcf..5712bcf 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -136,9 +136,13 @@
ddThe SCSI host number./dd
dtcodecapability/code/dt
ddCurrent capabilities include vports_ops (indicates
-vport operations are supported) and fc_host, the later
-implies following sub-elements: codewwnn/code,
-codewwpn/code, codefabric_wwn/code.
+vport operations are supported) and fc_host. vport_ops
+could contain two optional sub-elements: codevports/code,
+and codemax_vports/code. codevports/code shows the
+number of vport in use. codemax_vports/code shows the
+maximum vports the HBA supports. fc_host implies following
+sub-elements: codewwnn/code, codewwpn/code, and
+codefabric_wwn/code.
/dd
/dl
/dd
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 7c85815..b94cce6 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -267,6 +267,12 @@
attribute name='type'
valuevports_ops/value
/attribute
+element name='max_vports'
+ ref name='unsignedInt'/
+/element
+element name='vports'
+ ref name='unsignedInt'/
+/element
/define
define name='capscsihost'
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 819e6af..5962d58 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -392,7 +392,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr
def)
virBufferAddLit(buf, /capability\n);
}
if (data-scsi_host.flags VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)
{
-virBufferAddLit(buf, capability type='vport_ops'
/\n);
+virBufferAddLit(buf, capability type='vport_ops'\n);
+virBufferAsprintf(buf,
max_vports%d/max_vports\n,
+ data-scsi_host.max_vports);
+virBufferAsprintf(buf, vports%d/vports\n,
+ data-scsi_host.vports);
+virBufferAddLit(buf, /capability\n);
}
break;
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 145d699..4e584a3 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -137,6 +137,8 @@ struct _virNodeDevCapsDef {
char *wwpn;
char *fabric_wwn;
unsigned int flags;
+int max_vports;
+int vports;
} scsi_host;
struct {
char *name;
diff --git a/src/node_device/node_device_linux_sysfs.c
b/src/node_device/node_device_linux_sysfs.c
index 85bbab6..a1c3637 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -40,6 +40,8 @@
int
detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
{
+char *max_vports = NULL;
+char *vports = NULL;
int ret = -1;
VIR_DEBUG(Checking if host%d is an FC HBA, d-scsi_host.host);
@@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
d-scsi_host.host,
port_name,
- d-scsi_host.wwpn) == -1) {
+ d-scsi_host.wwpn) 0) {
VIR_ERROR(_(Failed to read WWPN for host%d),
d-scsi_host.host);
goto cleanup;
}
@@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
if (virReadFCHost(NULL,
d-scsi_host.host,
node_name,
- d-scsi_host.wwnn) == -1) {
+ d-scsi_host.wwnn) 0) {
VIR_ERROR(_(Failed to read WWNN for host%d),
Re: [libvirt] [PATCH 7/9 v2] nodedev: Fix the improper logic when enumerating SRIOV VF
On 15.01.2013 10:56, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no virtfn
entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions) ||
data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with virtual_function cap.
However, this results in a VF will aslo get virtual_function cap.
This patch fixes it by:
* Ignoring the VF which has failure of getting PCI config space
(given that the successfully detected VFs are kept , it makes
sense to not give up on the failure of one VF too) with a warning,
so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */
int ret = pciGetVirtualFunctions(syspath,
data-pci_dev.virtual_functions,
data-pci_dev.num_virtual_functions);
if (ret 0 )
goto out;
if (data-pci_dev.num_virtual_functions 0)
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
---
src/node_device/node_device_hal.c | 12 +++--
src/node_device/node_device_udev.c | 11 ++--
src/util/virpci.c | 46
+---
3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index e64d6ac..6da5873 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -151,9 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char
*udi,
if (!pciGetPhysicalFunction(sysfs_path,
d-pci_dev.physical_function))
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(sysfs_path,
d-pci_dev.virtual_functions,
-d-pci_dev.num_virtual_functions) ||
-d-pci_dev.num_virtual_functions 0)
+int ret = pciGetVirtualFunctions(sysfs_path,
+ d-pci_dev.virtual_functions,
+ d-pci_dev.num_virtual_functions);
+if (ret 0) {
+VIR_FREE(sysfs_path);
+return -1;
+}
+
+if (d-pci_dev.num_virtual_functions 0)
d-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path);
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 62f6320..a90217d 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device,
union _virNodeDevCapData *data = def-caps-data;
int ret = -1;
char *p;
+int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device,
if (!pciGetPhysicalFunction(syspath, data-pci_dev.physical_function))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-if (!pciGetVirtualFunctions(syspath, data-pci_dev.virtual_functions,
-data-pci_dev.num_virtual_functions) ||
-data-pci_dev.num_virtual_functions 0)
+rc = pciGetVirtualFunctions(syspath,
+data-pci_dev.virtual_functions,
+data-pci_dev.num_virtual_functions);
+/* Out of memory */
+if (rc 0)
+goto out;
+else if (!rc (data-pci_dev.num_virtual_functions 0))
data-pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0fb9923..ee4b3a8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path,
unsigned int *num_virtual_functions)
{
int ret = -1;
+int i;
DIR *dir = NULL;
struct dirent *entry = NULL;
char *device_link = NULL;
@@ -1989,45 +1990,52 @@ pciGetVirtualFunctions(const char *sysfs_path,
*num_virtual_functions = 0;
while ((entry = readdir(dir))) {
if (STRPREFIX(entry-d_name, virtfn)) {
+struct pci_config_address *config_addr = NULL;
if (virBuildPath(device_link, sysfs_path, entry-d_name) == -1)
{
virReportOOMError();
-goto out;
+goto error;
}
Re: [libvirt] the patch bridge: export multicast database via netlink broke kernel 3.8 uapi
Cong Wang skrev 15.1.2013 12:11: On Tue, 2013-01-15 at 12:03 +0200, Thomas Backlund wrote: Eric Blake skrev 15.1.2013 01:57: On 01/13/2013 01:05 PM, Thomas Backlund wrote: Thomas Backlund skrev 13.1.2013 20:38: patch both inline and attached as thunderbird tends to mess up ... - if_bridge.h uses struct in6_addr ip6; but does not include the in6.h header. Found by trying to build libvirt and connman against 3.8-rc3 headers. Ok, ignore this patch, it's not the correct fix as it introduces redefinitions... Btw, the error that I hit that made me suggest this fix was libvirt config check bailing out: config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has incomplete type Hmm, just now noticing this thread, after independently hitting and and having to work around the same problem in libvirt: https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html https://bugzilla.redhat.com/show_bug.cgi?id=895141 Yep, and the commit breaking uapi headers is by using struct in6_addr ip6 is: From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001 From: Cong Wang amw...@redhat.com Date: Fri, 7 Dec 2012 00:04:48 + Subject: [PATCH] bridge: export multicast database via netlink Does the following patch help? $ git diff include/uapi/linux/if_bridge.h diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h index 5db2975..653db23 100644 --- a/include/uapi/linux/if_bridge.h +++ b/include/uapi/linux/if_bridge.h @@ -14,6 +14,7 @@ #define _UAPI_LINUX_IF_BRIDGE_H #include linux/types.h +#include linux/in6.h #define SYSFS_BRIDGE_ATTR bridge #define SYSFS_BRIDGE_FDB brforward Well, I suggested the same fix in the beginning of the thread on netdev and lkml: if_bridge.h: include in6.h for struct in6_addr use as it seemed to fix the libvirt case but then asked it to be ignored after I tried to build connman, and hit this conflict with glibc-2.17: In file included from /usr/include/arpa/inet.h:22:0, from ./include/connman/inet.h:25, from src/connman.h:128, from src/tethering.c:40: /usr/include/netinet/in.h:35:5: error: expected identifier before numeric constant /usr/include/netinet/in.h:197:8: error: redefinition of 'struct in6_addr' In file included from /usr/include/linux/if_bridge.h:17:0, from src/tethering.c:38: /usr/include/linux/in6.h:30:8: note: originally defined here In file included from /usr/include/arpa/inet.h:22:0, from ./include/connman/inet.h:25, from src/connman.h:128, from src/tethering.c:40: /usr/include/netinet/in.h:238:8: error: redefinition of 'struct sockaddr_in6' In file included from /usr/include/linux/if_bridge.h:17:0, from src/tethering.c:38: /usr/include/linux/in6.h:46:8: note: originally defined here In file included from /usr/include/arpa/inet.h:22:0, from ./include/connman/inet.h:25, from src/connman.h:128, from src/tethering.c:40: /usr/include/netinet/in.h:274:8: error: redefinition of 'struct ipv6_mreq' In file included from /usr/include/linux/if_bridge.h:17:0, from src/tethering.c:38: /usr/include/linux/in6.h:54:8: note: originally defined here make[1]: *** [src/src_connmand-tethering.o] Error 1 So I'm not sure it's the right one... -- Thomas -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 13/14] storage: Need to also VIR_FREE(reg)
On 01/10/13 20:44, John Ferlan wrote: Commit-id 'afc4631b' added the regfree(reg) to free resources alloc'd during regcomp; however, reg still needed to be VIR_FREE()'d. The call to regfree() also didn't account for possible NULL value. Reformatted the call to be closer to usage. --- src/storage/storage_backend_logical.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Resolve UNINIT errors found by Coverity
On 01/08/13 16:40, John Ferlan wrote: This set of patches resolves Error: UNINIT (CWE-457) errors found by Coverity John Ferlan (4): parallels: Resolve issues with uninitialized 'ret' value openvz: Need to initialize 'ret' for kb_per_pages error path lxc: Initialize dst due to potential cleanup usage before setting interface: Need to initialize 'add_to_list' src/interface/interface_backend_udev.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_storage.c | 19 ++- 4 files changed, 17 insertions(+), 8 deletions(-) Pushed now. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 13/14] storage: Need to also VIR_FREE(reg)
On 01/15/13 12:07, Ján Tomko wrote: On 01/10/13 20:44, John Ferlan wrote: Commit-id 'afc4631b' added the regfree(reg) to free resources alloc'd during regcomp; however, reg still needed to be VIR_FREE()'d. The call to regfree() also didn't account for possible NULL value. Reformatted the call to be closer to usage. --- src/storage/storage_backend_logical.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) ACK And pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4 v3] New API virNodeDeviceLookupSCSIHostByWWN
On 2013年01月14日 11:09, Osier Yang wrote: For easy reviewing, this is splitted from [1]. Ping, anyone can review this? v2 - v3: * Validate the specified wwnn,wwpn pair before applying it to the new API in virsh-nodedev.c v1 - v2: * Per Daniel's suggestion, change the API name from virNodeDeviceLookupByWWN into virNodeDeviceLookupSCSIHostByWWN. Osier Yang (4): Introduce API virNodeDeviceLookupSCSIHostByWWN remote: Wire up the remote protocol nodedev: Implement virNodeDeviceLookupSCSIHostByWWN virsh: Use virNodeDeviceLookupSCSIHostByWWN include/libvirt/libvirt.h.in |5 ++ src/driver.h |6 ++ src/libvirt.c| 46 src/libvirt_public.syms |1 + src/node_device/node_device_driver.c | 13 +++-- src/node_device/node_device_driver.h |4 ++ src/node_device/node_device_hal.c|1 + src/node_device/node_device_udev.c |1 + src/remote/remote_driver.c |1 + src/remote/remote_protocol.x | 13 - src/remote_protocol-structs |9 +++ src/rpc/gendispatch.pl |5 ++- tools/virsh-nodedev.c| 97 ++ tools/virsh.pod | 15 +++-- 14 files changed, 181 insertions(+), 36 deletions(-) [1] https://www.redhat.com/archives/libvir-list/2013-January/msg00328.html Regards, Osier -- 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] qemu: fix QEMU_CAPS_NO_ACPI detection
On 12/21/12 14:38, Ján Tomko wrote: In commit c4bbaaf8, caps-arch was checked uninitialized, rendering the whole check useless. This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to qemuCapsInitQMP, and removes the no longer needed exception for S390. It also clears the flag for all non-x86 archs instead of just S390 in qemuCapsInitHelp. --- src/qemu/qemu_capabilities.c | 29 - 1 files changed, 8 insertions(+), 21 deletions(-) gentle ping -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix QEMU_CAPS_NO_ACPI detection
On 2013年01月15日 19:47, Ján Tomko wrote: On 12/21/12 14:38, Ján Tomko wrote: In commit c4bbaaf8, caps-arch was checked uninitialized, rendering the whole check useless. This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to qemuCapsInitQMP, and removes the no longer needed exception for S390. It also clears the flag for all non-x86 archs instead of just S390 in qemuCapsInitHelp. --- src/qemu/qemu_capabilities.c | 29 - 1 files changed, 8 insertions(+), 21 deletions(-) gentle ping This looks good for PPC. :) Thanks. -- 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
[libvirt] [PATCH] .gitignore: Sort alphabetically
--- Pushed under trivial rule .gitignore | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index f71c30c..be83e28 100644 --- a/.gitignore +++ b/.gitignore @@ -63,8 +63,8 @@ /docs/devhelp/libvirt.devhelp /docs/hvsupport.html.in /docs/libvirt-api.xml -/docs/libvirt-qemu-*.xml /docs/libvirt-lxc-*.xml +/docs/libvirt-qemu-*.xml /docs/libvirt-refs.xml /docs/search.php /docs/todo.html.in @@ -161,8 +161,8 @@ /tests/reconnect /tests/secaatest /tests/seclabeltest -/tests/securityselinuxtest /tests/securityselinuxlabeltest +/tests/securityselinuxtest /tests/sexpr2xmltest /tests/shunloadtest /tests/sockettest -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/14] locking: Resolve resource leak with 'dir' on non error path
On 01/09/13 15:54, John Ferlan wrote: --- src/locking/lock_driver_sanlock.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index d06fa66..df6bee9 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -235,6 +235,7 @@ static int virLockManagerSanlockSetupLockspace(void) path); goto error; } +VIR_FREE(dir); if (driver-group != (gid_t) -1) perms |= 0060; That does fix the 'dir' leak, but it seems we still leak 'path' on non-error path this way. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/14] test: Resource resource leak with 'tmp_vols'
On 01/09/13 15:54, John Ferlan wrote: --- src/test/test_driver.c | 1 + 1 file changed, 1 insertion(+) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/14] interface: Resolve resource leak wth 'tmp_iface_objs'
On 01/09/13 15:54, John Ferlan wrote: --- src/interface/interface_backend_netcf.c | 1 + 1 file changed, 1 insertion(+) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/14] util: Resolve resource leak for 'res' in virSetInherit error path.
On 01/09/13 15:54, John Ferlan wrote: --- src/util/virlockspace.c | 1 + 1 file changed, 1 insertion(+) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/14] storage: Resource resource leak using 'tmp_vols'
On 01/09/13 15:54, John Ferlan wrote: --- src/storage/storage_driver.c | 1 + 1 file changed, 1 insertion(+) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/14] rpc: Avoid resource leak of 'socks' if any object append fails
On 01/09/13 15:54, John Ferlan wrote: --- src/rpc/virnetserverservice.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/14] Resolve some RESOURCE_LEAK errors found by Coverity
On 01/14/13 16:54, John Ferlan wrote: ping on 4 - 13 1 3 were pushed 2 is replaced by https://www.redhat.com/archives/libvir-list/2013-January/msg00657.html 14 is replaced by https://www.redhat.com/archives/libvir-list/2013-January/msg00658.html I've ACKed and pushed 4, 6, 8-10, 13 and pushed 11. That leaves 5, 7 and 12. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/14] locking: Resolve resource leaks on non error path
Both 'dir' and 'path' were not free'd on successful return --- src/locking/lock_driver_sanlock.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index d06fa66..0b7c6d5 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -365,6 +365,8 @@ retry: VIR_DEBUG(Lockspace %s has been registered, path); } +VIR_FREE(path); +VIR_FREE(dir); return 0; error_unlink: -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Add total number of physical cores to the capabilities output
https://bugzilla.redhat.com/show_bug.cgi?id=888503 The values added by this patch help avoid management applications the dodgy approach of multiplying values in the nodeinfo structure to gather data needed for their management decisions. Peter Krempa (2): nodeinfo: Gather total number of physical cores and fix tests capabilities: Add total number of cores and threads to the XML docs/formatcaps.html.in | 2 +- docs/schemas/capability.rng | 8 +++ src/conf/cpu_conf.c | 4 src/conf/cpu_conf.h | 2 ++ src/libvirt_private.syms| 1 + src/nodeinfo.c | 37 ++--- src/nodeinfo.h | 2 ++ src/qemu/qemu_capabilities.c| 3 ++- tests/nodeinfodata/linux-x86-test1.expected | 2 +- tests/nodeinfodata/linux-x86-test2.expected | 2 +- tests/nodeinfodata/linux-x86-test3.expected | 2 +- tests/nodeinfodata/linux-x86-test4.expected | 2 +- tests/nodeinfodata/linux-x86-test5.expected | 2 +- tests/nodeinfodata/linux-x86-test6.expected | 2 +- tests/nodeinfodata/linux-x86-test7.expected | 2 +- tests/nodeinfodata/linux-x86-test8.expected | 2 +- tests/nodeinfotest.c| 11 + tests/testutilsqemu.c | 2 ++ 18 files changed, 66 insertions(+), 22 deletions(-) -- 1.8.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] nodeinfo: Gather total number of physical cores and fix tests
Gather the total number of physical cores needed for the next patch and
add the data to the tests.
The incorrect value in test 7 is expected as the test is based on data
from a machine that has incorrect NUMA information.
---
src/libvirt_private.syms| 1 +
src/nodeinfo.c | 37 ++---
src/nodeinfo.h | 2 ++
tests/nodeinfodata/linux-x86-test1.expected | 2 +-
tests/nodeinfodata/linux-x86-test2.expected | 2 +-
tests/nodeinfodata/linux-x86-test3.expected | 2 +-
tests/nodeinfodata/linux-x86-test4.expected | 2 +-
tests/nodeinfodata/linux-x86-test5.expected | 2 +-
tests/nodeinfodata/linux-x86-test6.expected | 2 +-
tests/nodeinfodata/linux-x86-test7.expected | 2 +-
tests/nodeinfodata/linux-x86-test8.expected | 2 +-
tests/nodeinfotest.c| 11 +
12 files changed, 47 insertions(+), 20 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7be58ee..031937f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -918,6 +918,7 @@ nodeGetCPUMap;
nodeGetCPUStats;
nodeGetFreeMemory;
nodeGetInfo;
+nodeGetInfoCores;
nodeGetMemoryParameters;
nodeGetMemoryStats;
nodeSetMemoryParameters;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 477104f..b21fc3a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -90,7 +90,8 @@ freebsdNodeGetCPUCount(void)
/* NB, this is not static as we need to call it from the testsuite */
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
const char *sysfs_dir,
- virNodeInfoPtr nodeinfo);
+ virNodeInfoPtr nodeinfo,
+ unsigned int *totalcores);
static int linuxNodeGetCPUStats(FILE *procstat,
int cpuNum,
@@ -228,12 +229,13 @@ CPU_COUNT(cpu_set_t *set)
static int
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
-ATTRIBUTE_NONNULL(5)
+ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
virNodeParseNode(const char *node,
int *sockets,
int *cores,
int *threads,
- int *offline)
+ int *offline,
+ unsigned int *totalcores)
{
int ret = -1;
int processors = 0;
@@ -357,6 +359,7 @@ virNodeParseNode(const char *node,
continue;
core = CPU_COUNT(core_maps[i]);
+*totalcores += core;
if (core *cores)
*cores = core;
}
@@ -376,7 +379,8 @@ cleanup:
int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
const char *sysfs_dir,
- virNodeInfoPtr nodeinfo)
+ virNodeInfoPtr nodeinfo,
+ unsigned int *totalcoresinfo)
{
char line[1024];
DIR *nodedir = NULL;
@@ -386,6 +390,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
int ret = -1;
char *sysfs_nodedir = NULL;
char *sysfs_cpudir = NULL;
+unsigned int totalcores = 0;
nodeinfo-cpus = 0;
nodeinfo-mhz = 0;
@@ -503,7 +508,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
}
if ((cpus = virNodeParseNode(sysfs_cpudir, socks, cores,
- threads, offline)) 0)
+ threads, offline, totalcores)) 0)
goto cleanup;
VIR_FREE(sysfs_cpudir);
@@ -538,8 +543,9 @@ fallback:
goto cleanup;
}
+totalcores = 0;
if ((cpus = virNodeParseNode(sysfs_cpudir, socks, cores,
- threads, offline)) 0)
+ threads, offline, totalcores)) 0)
goto cleanup;
nodeinfo-nodes = 1;
@@ -582,6 +588,9 @@ done:
nodeinfo-threads = 1;
}
+if (totalcoresinfo)
+*totalcoresinfo = totalcores;
+
ret = 0;
cleanup:
@@ -864,7 +873,10 @@ error:
}
#endif
-int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo)
+
+int
+nodeGetInfoCores(virNodeInfoPtr nodeinfo,
+ unsigned int *totalcores)
{
virArch hostarch = virArchFromHost();
@@ -881,7 +893,8 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED,
virNodeInfoPtr nodeinfo)
return -1;
}
-ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo);
+ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH,
+ nodeinfo, totalcores);
if (ret 0)
goto cleanup;
@@ -936,6 +949,14 @@ cleanup:
#endif
}
+
+int
+nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo)
+{
+return nodeGetInfoCores(nodeinfo, NULL);
+}
+
+
int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
int cpuNum ATTRIBUTE_UNUSED,
virNodeCPUStatsPtr params ATTRIBUTE_UNUSED,
diff --git a/src/nodeinfo.h
[libvirt] [PATCH] conf: fix leak in virDomainVcpuPinAdd
Fix the leak of vcpupin on failure to allocate cpumask and the leak of
cpumask if we fail to expand vcpupin_list.
---
src/conf/domain_conf.c | 25 -
1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6feded4..95ecd9d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11908,26 +11908,25 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr
**vcpupin_list,
/* No existing vcpupin matches vcpu, adding a new one */
-if (VIR_ALLOC(vcpupin) 0) {
-virReportOOMError();
-return -1;
-}
+if (VIR_ALLOC(vcpupin) 0)
+goto no_memory;
+
vcpupin-vcpuid = vcpu;
vcpupin-cpumask = virBitmapNewData(cpumap, maplen);
-if (!vcpupin-cpumask) {
-virReportOOMError();
-return -1;
-}
+if (!vcpupin-cpumask)
+goto no_memory;
-if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) 0) {
-virReportOOMError();
-VIR_FREE(vcpupin);
-return -1;
-}
+if (VIR_REALLOC_N(*vcpupin_list, *nvcpupin + 1) 0)
+goto no_memory;
(*vcpupin_list)[(*nvcpupin)++] = vcpupin;
return 0;
+
+no_memory:
+virReportOOMError();
+virDomainVcpuPinDefFree(vpcupin);
+return -1;
}
int
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix class_id bitmap leak in virNetworkObj
Commit '07d1b6b' added class_id bitmap to virNetworkObj but never freed it. --- src/conf/network_conf.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 48639a9..9c35ea8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -235,6 +235,7 @@ void virNetworkObjFree(virNetworkObjPtr net) virNetworkDefFree(net-def); virNetworkDefFree(net-newDef); +virBitmapFree(net-class_id); virMutexDestroy(net-lock); -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] capabilities: Add total number of cores and threads to the XML
On Tue, Jan 15, 2013 at 03:45:24PM +0100, Peter Krempa wrote: Management applications may want to limit the maximum number of vCPUs the guest has assigned based on the number of physical cores in the system (excluding threads) for performance reasons. This patch adds output of the total number of cores and total number of threads present in the system as this information couldn't be reliably determined in all cases by the data provided by libvirt. The new output looks like this on a system with HyperThreading: capabilities host cpu archx86_64/arch modelSandyBridge/model vendorIntel/vendor topology sockets='1' cores='2' threads='2' totalcores='2' totalthreads='4'/ NACK, we should not be adding this data here - it is duplicating info better provide in the NUMA topology hierarchy. Please explain why this is not already sufficient ? Regards, 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] the patch bridge: export multicast database via netlink broke kernel 3.8 uapi (was: Re: if_bridge.h: include in6.h for struct in6_addr use)
On Tue, 2013-01-15 at 12:03 +0200, Thomas Backlund wrote: Eric Blake skrev 15.1.2013 01:57: On 01/13/2013 01:05 PM, Thomas Backlund wrote: Thomas Backlund skrev 13.1.2013 20:38: patch both inline and attached as thunderbird tends to mess up ... - if_bridge.h uses struct in6_addr ip6; but does not include the in6.h header. Found by trying to build libvirt and connman against 3.8-rc3 headers. Ok, ignore this patch, it's not the correct fix as it introduces redefinitions... Btw, the error that I hit that made me suggest this fix was libvirt config check bailing out: config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has incomplete type Hmm, just now noticing this thread, after independently hitting and and having to work around the same problem in libvirt: https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html https://bugzilla.redhat.com/show_bug.cgi?id=895141 Yep, and the commit breaking uapi headers is by using struct in6_addr ip6 is: From ee07c6e7a6f8a25c18f0a6b18152fbd7499245f6 Mon Sep 17 00:00:00 2001 From: Cong Wang amw...@redhat.com Date: Fri, 7 Dec 2012 00:04:48 + Subject: [PATCH] bridge: export multicast database via netlink Does the following patch help? $ git diff include/uapi/linux/if_bridge.h diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h index 5db2975..653db23 100644 --- a/include/uapi/linux/if_bridge.h +++ b/include/uapi/linux/if_bridge.h @@ -14,6 +14,7 @@ #define _UAPI_LINUX_IF_BRIDGE_H #include linux/types.h +#include linux/in6.h #define SYSFS_BRIDGE_ATTR bridge #define SYSFS_BRIDGE_FDB brforward -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't update count of vCPUs if hot-unplug has failed
After live change of cpu counts, the number of processor threads is
verified. This patch makes use of this approach to check if qemu ignored
the request for cpu hot-unplug and report an appropriate message.
---
src/qemu/qemu_driver.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 39175f4..c4be130 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3697,6 +3697,15 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr
driver,
goto cleanup;
}
+/* check if hotplug has failed */
+if (vcpus oldvcpus ncpupids == oldvcpus) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(qemu didn't unplug the vCPUs properly));
+vcpus = oldvcpus;
+ret = -1;
+goto cleanup;
+}
+
if (ncpupids != vcpus) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(got wrong number of vCPU pids from QEMU monitor.
--
1.8.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix class_id bitmap leak in virNetworkObj
On 01/15/2013 09:48 AM, Ján Tomko wrote: Commit '07d1b6b' added class_id bitmap to virNetworkObj but never freed it. --- src/conf/network_conf.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 48639a9..9c35ea8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -235,6 +235,7 @@ void virNetworkObjFree(virNetworkObjPtr net) virNetworkDefFree(net-def); virNetworkDefFree(net-newDef); +virBitmapFree(net-class_id); virMutexDestroy(net-lock); ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix leak in virDomainVcpuPinAdd
On 01/15/2013 09:48 AM, Ján Tomko wrote: Fix the leak of vcpupin on failure to allocate cpumask and the leak of cpumask if we fail to expand vcpupin_list. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/8] *Persistent vHBA support in storage pool
This is the 3rd part to implement NPIV migration support [1]. Part 1: https://www.redhat.com/archives/libvir-list/2013-January/msg00859.html Part 2: (Already ACKed by Michal) https://www.redhat.com/archives/libvir-list/2013-January/msg00859.html The patches are based on Part 2. The new XMLs might be too long, might be deserved to have them as sub-elements of adapter. But I'd like to see the feedback first. Osier Yang (8): New XML attributes for storage pool source adapter storage: Make the adapter name be consistent with node device driver storage: Move virStorageBackendSCSIGetHostNumber into iscsi backend phyp: Prohibit fc_host adapter for phyp driver util: Add helper to get the scsi host name by iterating over sysfs util: Fix bug of managing vport storage: Add startPool and stopPool for scsi backend storage: Guess the parent if it's not specified for vHBA docs/formatstorage.html.in | 15 ++- docs/schemas/storagepool.rng | 33 +- src/conf/storage_conf.c| 123 -- src/conf/storage_conf.h| 23 +++- src/libvirt_private.syms |4 + src/phyp/phyp_driver.c | 15 ++- src/storage/storage_backend_iscsi.c| 39 ++- src/storage/storage_backend_scsi.c | 190 +++ src/storage/storage_backend_scsi.h |3 - src/util/virutil.c | 196 +++- src/util/virutil.h |7 + tests/storagepoolxml2xmlin/pool-scsi.xml |2 +- tests/storagepoolxml2xmlin/pool-scsi1.xml | 15 ++ tests/storagepoolxml2xmlout/pool-scsi.xml |2 +- tests/storagepoolxml2xmlout/pool-scsi1.xml | 18 +++ tests/storagepoolxml2xmltest.c |1 + 16 files changed, 602 insertions(+), 84 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-scsi1.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi1.xml [1] https://www.redhat.com/archives/libvir-list/2012-November/msg00826.html Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/8] storage: Add startPool and stopPool for scsi backend
startPool creates the vHBA if it's not existed yet, stopPool destroys
the vHBA. Also to support autostart, checkPool will creates the vHBA
if it's not existed yet.
---
src/storage/storage_backend_scsi.c | 64
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 8166311..a9b96a3 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -634,6 +634,36 @@ getAdapterName(virStoragePoolSourceAdapter adapter)
}
static int
+createVport(virStoragePoolSourceAdapter adapter)
+{
+int parent_host;
+
+if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
+return 0;
+
+/* This filters either HBA or already created vHBA */
+if (virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn,
+ adapter.data.fchost.wwpn))
+return 0;
+
+if (!adapter.data.fchost.parent) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _('parent' for vHBA must be specified));
+return -1;
+}
+
+if ((parent_host = getHostNumber(adapter.data.fchost.parent)) 0)
+return -1;
+
+if (virManageVport(parent_host, adapter.data.fchost.wwnn,
+ adapter.data.fchost.wwpn, VPORT_CREATE) 0)
+return -1;
+
+virFileWaitForDevices();
+return 0;
+}
+
+static int
virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
bool *isActive)
@@ -695,10 +725,44 @@ out:
return ret;
}
+static int
+virStorageBackendSCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool)
+{
+virStoragePoolSourceAdapter adapter = pool-def-source.adapter;
+
+return createVport(adapter);
+}
+
+static int
+virStorageBackendSCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool)
+{
+virStoragePoolSourceAdapter adapter = pool-def-source.adapter;
+int parent_host;
+
+if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
+return 0;
+
+if (!(virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn,
+adapter.data.fchost.wwpn)))
+return -1;
+
+if ((parent_host = getHostNumber(adapter.data.fchost.parent)) 0)
+return -1;
+
+if (virManageVport(parent_host, adapter.data.fchost.wwnn,
+ adapter.data.fchost.wwpn, VPORT_DELETE) 0)
+return -1;
+
+return 0;
+}
virStorageBackend virStorageBackendSCSI = {
.type = VIR_STORAGE_POOL_SCSI,
.checkPool = virStorageBackendSCSICheckPool,
.refreshPool = virStorageBackendSCSIRefreshPool,
+.startPool = virStorageBackendSCSIStartPool,
+.stopPool = virStorageBackendSCSIStopPool,
};
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/8] storage: Guess the parent if it's not specified for vHBA
This finds the parent for vHBA by iterating over all the HBA
which supports vport_ops capability on the host, and return
the first one which is online, not saturated (vports in use
is less than max_vports).
---
docs/formatstorage.html.in |3 +-
src/libvirt_private.syms |1 +
src/storage/storage_backend_scsi.c | 10 +++--
src/util/virutil.c | 83
src/util/virutil.h |2 +
5 files changed, 93 insertions(+), 6 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index af42fed..7315865 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -99,8 +99,7 @@
codewwpn/code (span class=since1.0.2/span) indicates
the (v)HBA. The optional attribute codeparent/code
(span class=since1.0.2/span) specifies the parent device of
-the (v)HBA. It's optional for HBA, but required for vHBA.
-span class=sinceSince 0.6.2/span/dd
+the (v)HBA. span class=sinceSince 0.6.2/span/dd
dtcodehost/code/dt
ddProvides the source for pools backed by storage from a
remote server. Will be used in combination with a
codedirectory/code
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0bec5a0..96059fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1829,6 +1829,7 @@ virFileStripSuffix;
virFileUnlock;
virFileWaitForDevices;
virFileWriteStr;
+virFindFCHostCapableVport;
virFindFileInPath;
virFormatIntDecimal;
virGetDeviceID;
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index a9b96a3..59abeb0 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -646,10 +646,12 @@ createVport(virStoragePoolSourceAdapter adapter)
adapter.data.fchost.wwpn))
return 0;
-if (!adapter.data.fchost.parent) {
-virReportError(VIR_ERR_XML_ERROR, %s,
- _('parent' for vHBA must be specified));
-return -1;
+if (!adapter.data.fchost.parent
+!(adapter.data.fchost.parent = virFindFCHostCapableVport(NULL))) {
+ virReportError(VIR_ERR_XML_ERROR, %s,
+ _('parent' for vHBA not specified, and
+ cannot find one on this host));
+ return -1;
}
if ((parent_host = getHostNumber(adapter.data.fchost.parent)) 0)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 948cea9..35195a5 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -3547,6 +3547,82 @@ cleanup:
VIR_FREE(wwpn_buf);
return ret;
}
+
+# define PORT_STATE_ONLINE Online
+
+/* virFindFCHostCapableVport:
+ *
+ * Iterate over the sysfs and find out the first online HBA which
+ * supports vport, and not saturated,.
+ */
+char *
+virFindFCHostCapableVport(const char *sysfs_prefix)
+{
+const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH;
+DIR *dir = NULL;
+struct dirent *entry = NULL;
+char *max_vports = NULL;
+char *vports = NULL;
+char *state = NULL;
+char *ret = NULL;
+
+if (!(dir = opendir(prefix))) {
+virReportSystemError(errno,
+ _(Failed to opendir path '%s'),
+ prefix);
+return NULL;
+}
+
+while ((entry = readdir(dir))) {
+if (STREQ(entry-d_name, .) || STREQ(entry-d_name, ..))
+continue;
+
+int host;
+
+ignore_value(sscanf(entry-d_name, host%d, host));
+if (!virIsCapableVport(NULL, host))
+continue;
+
+if (virReadFCHost(NULL, host, port_state, state) 0) {
+ VIR_DEBUG(Failed to read port_state for host%d, host);
+ continue;
+}
+
+/* Skip the not online FC host */
+if (!STREQ(state, PORT_STATE_ONLINE)) {
+VIR_FREE(state);
+continue;
+}
+VIR_FREE(state);
+
+if (virReadFCHost(NULL, host, max_npiv_vports, max_vports) 0) {
+ VIR_DEBUG(Failed to read max_npiv_vports for host%d, host);
+ continue;
+}
+
+if (virReadFCHost(NULL, host, npiv_vports_inuse, vports) 0) {
+ VIR_DEBUG(Failed to read npiv_vports_inuse for host%d, host);
+ VIR_FREE(max_vports);
+ continue;
+}
+
+if ((strlen(max_vports) strlen(vports)) ||
+((strlen(max_vports) == strlen(vports))
+ strcmp(max_vports, vports) 0)) {
+ret = strdup(entry-d_name);
+goto cleanup;
+}
+
+VIR_FREE(max_vports);
+VIR_FREE(vports);
+}
+
+cleanup:
+closedir(dir);
+VIR_FREE(max_vports);
+VIR_FREE(vports);
+return ret;
+}
#else
int
virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
@@ -3592,4 +3668,11 @@ virGetFCHostNameByWWN(const char *sysfs_prefix
ATTRIBUTE_UNUSED,
[libvirt] [PATCH 1/8] New XML attributes for storage pool source adapter
This introduces 4 new attributes for storage pool source adapter. E.g. adapter type='fc_host' parent='scsi_host5' wwnn='2000c9831b4b' wwpn='1000c9831b4b'/ Attribute 'type' can be either 'scsi_host' or 'fc_host', and defaults to 'scsi_host' if attribute 'name' is specified. I.e. It's optional for 'scsi_host' adapter, for back-compact reason. However, it's mandatory for 'fc_host' adapter and any new future adapter types. Attribute 'parent' is required for vHBA, but optional for HBA. * docs/formatstorage.html.in: - Add documents for the 4 new attrs * docs/schemas/storagepool.rng: - Add RNG schema * src/conf/storage_conf.c: - Parse and format the new XMLs * src/conf/storage_conf.h: - New struct virStoragePoolSourceAdapter, replace char *adapter with it; - New enum virStoragePoolSourceAdapterType * src/libvirt_private.syms: - Export TypeToString and TypeFromString * src/phyp/phyp_driver.c: - Replace adapter with adapter.data.name, which is member of the union of the new struct virStoragePoolSourceAdapter now * src/storage/storage_backend_scsi.c: - Like above * tests/storagepoolxml2xmlin/pool-scsi1.xml: - New test for 'fc_host' adapter * tests/storagepoolxml2xmlout/pool-scsi.xml: - Change the expected output, as the 'type' defaults to 'scsi_host' if 'name specified now * tests/storagepoolxml2xmlout/pool-scsi1.xml: - New test * tests/storagepoolxml2xmltest.c: - Include the test --- docs/formatstorage.html.in | 13 +++- docs/schemas/storagepool.rng | 33 +++- src/conf/storage_conf.c| 123 +-- src/conf/storage_conf.h| 23 +- src/libvirt_private.syms |2 + src/phyp/phyp_driver.c |8 +- src/storage/storage_backend_scsi.c |6 +- tests/storagepoolxml2xmlin/pool-scsi1.xml | 15 tests/storagepoolxml2xmlout/pool-scsi.xml |2 +- tests/storagepoolxml2xmlout/pool-scsi1.xml | 18 tests/storagepoolxml2xmltest.c |1 + 11 files changed, 220 insertions(+), 24 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-scsi1.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi1.xml diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 9f93db8..0849618 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -88,8 +88,17 @@ span class=sinceSince 0.4.1/span/dd dtcodeadapter/code/dt ddProvides the source for pools backed by SCSI adapters. May -only occur once. Contains a single attribute codename/code -which is the SCSI adapter name (ex. host1). +only occur once. Attribute codename/code is the SCSI adapter +name (ex. host1). Attribute codetype/code +(span class=since1.0.2/span) specifies the adapter type, +valid value is fc_host or scsi_host, defaults to scsi_host +if codename/code is specified. To keep back-compact, +codetype/code is optional for scsi_host adapter, but +mandatory for fc_host adapter. Attribute codewwnn/code and +codewwpn/code (span class=since1.0.2/span) indicates +the (v)HBA. The optional attribute codeparent/code +(span class=since1.0.2/span) specifies the parent device of +the (v)HBA. It's optional for HBA, but required for vHBA. span class=sinceSince 0.6.2/span/dd dtcodehost/code/dt ddProvides the source for pools backed by storage from a diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 983f664..73e8ff7 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -275,9 +275,36 @@ define name='sourceinfoadapter' element name='adapter' - attribute name='name' -text/ - /attribute + choice +group + !-- To keep back-compact, 'type' is not mandatory for + scsi_host adapter -- + optional +attribute name='type' + valuescsi_host/value +/attribute + /optional + attribute name='name' +text/ + /attribute +/group +group + attribute name='type' +valuefc_host/value + /attribute + optional +attribute name='parent' + text/ +/attribute + /optional + attribute name='wwnn' +ref name='wwn'/ + /attribute + attribute name='wwpn' +ref name='wwn'/ + /attribute +/group + /choice empty/ /element /define diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 7a39998..ddb2945 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -90,6 +90,10 @@ VIR_ENUM_IMPL(virStoragePartedFsType, ext2, ext2, extended) +VIR_ENUM_IMPL(virStoragePoolSourceAdapterType, +
[libvirt] [PATCH 4/8] phyp: Prohibit fc_host adapter for phyp driver
It's possible to support fc_host adapter for phyp driver too, but
at this stage I'd like to not allow it when I'm not that clear
how it works.
---
src/phyp/phyp_driver.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index a12a430..c359048 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -2515,6 +2515,13 @@ phypBuildStoragePool(virConnectPtr conn,
virStoragePoolDefPtr def)
int exit_status = 0;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+if (source.adapter.type !=
+VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Only 'scsi_host' adapter is supported));
+goto cleanup;
+}
+
if (system_type == HMC)
virBufferAsprintf(buf, viosvrcmd -m %s --id %d -c ',
managed_system, vios_id);
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/8] util: Fix bug of managing vport
The string written to vport_create or vport_delete should
be wwnn:wwpn, but not wwpn:wwnn.
---
src/util/virutil.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 4171004..948cea9 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -3429,8 +3429,8 @@ virManageVport(const int parent_host,
if (virAsprintf(vport_name,
%s:%s,
-wwpn,
-wwnn) 0) {
+wwnn,
+wwpn) 0) {
virReportOOMError();
goto cleanup;
}
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/8] storage: Make the adapter name be consistent with node device driver
node device driver names the HBA like scsi_host5, but storage
driver uses host5, which could make the user confused. This
make them consistent. However, for back-compact reason, adapter
name like host5 is still supported.
---
docs/formatstorage.html.in| 13 +
src/storage/storage_backend_scsi.c| 43 +
tests/storagepoolxml2xmlin/pool-scsi.xml |2 +-
tests/storagepoolxml2xmlout/pool-scsi.xml |2 +-
4 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 0849618..af42fed 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -89,12 +89,13 @@
dtcodeadapter/code/dt
ddProvides the source for pools backed by SCSI adapters. May
only occur once. Attribute codename/code is the SCSI adapter
-name (ex. host1). Attribute codetype/code
-(span class=since1.0.2/span) specifies the adapter type,
-valid value is fc_host or scsi_host, defaults to scsi_host
-if codename/code is specified. To keep back-compact,
-codetype/code is optional for scsi_host adapter, but
-mandatory for fc_host adapter. Attribute codewwnn/code and
+name (ex. scsi_host1, name like 'host1' is not recommended to
+use, though it's still supported for back-compact reason).
+Attribute codetype/code (span class=since1.0.2/span)
+specifies the adapter type, valid value is fc_host or scsi_host,
+defaults to scsi_host if codename/code is specified. To keep
+back-compact, codetype/code is optional for scsi_host adapter,
+but mandatory for fc_host adapter. Attribute codewwnn/code and
codewwpn/code (span class=since1.0.2/span) indicates
the (v)HBA. The optional attribute codeparent/code
(span class=since1.0.2/span) specifies the parent device of
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index c1c163e..a26bf59 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -632,14 +632,38 @@ out:
}
static int
+getHostNumber(const char *adapter_name)
+{
+int host;
+
+/* Specifying adpater like 'host5' is still supported for
+ * back-compact reason.
+ */
+if ((sscanf(adapter_name, scsi_host%d, host) != 1)
+(sscanf(adapter_name, host%d, host) != 1)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to get host number from '%s'),
+ adapter_name);
+return -1;
+}
+
+return host;
+}
+
+static int
virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
bool *isActive)
{
char *path;
+int host;
*isActive = false;
-if (virAsprintf(path, /sys/class/scsi_host/%s,
pool-def-source.adapter.data.name) 0) {
+
+if ((host = getHostNumber(pool-def-source.adapter.data.name)) 0)
+return -1;
+
+if (virAsprintf(path, /sys/class/scsi_host/host%d, host) 0) {
virReportOOMError();
return -1;
}
@@ -656,29 +680,24 @@ static int
virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool)
{
-int retval = 0;
-uint32_t host;
+int ret = -1;
+int host;
pool-def-allocation = pool-def-capacity = pool-def-available = 0;
-if (sscanf(pool-def-source.adapter.data.name, host%u, host) != 1) {
-VIR_DEBUG(Failed to get host number from '%s',
-pool-def-source.adapter.data.name);
-retval = -1;
+if ((host = getHostNumber(pool-def-source.adapter.data.name)) 0)
goto out;
-}
VIR_DEBUG(Scanning host%u, host);
-if (virStorageBackendSCSITriggerRescan(host) 0) {
-retval = -1;
+if (virStorageBackendSCSITriggerRescan(host) 0)
goto out;
-}
virStorageBackendSCSIFindLUs(pool, host);
+ret = 0;
out:
-return retval;
+return ret;
}
diff --git a/tests/storagepoolxml2xmlin/pool-scsi.xml
b/tests/storagepoolxml2xmlin/pool-scsi.xml
index 3650e43..091ecfc 100644
--- a/tests/storagepoolxml2xmlin/pool-scsi.xml
+++ b/tests/storagepoolxml2xmlin/pool-scsi.xml
@@ -2,7 +2,7 @@
namehba0/name
uuide9392370-2917-565e-692b-d057f46512d6/uuid
source
-adapter name=host0/
+adapter name=scsi_host0/
/source
target
path/dev/disk/by-path/path
diff --git a/tests/storagepoolxml2xmlout/pool-scsi.xml
b/tests/storagepoolxml2xmlout/pool-scsi.xml
index faf5342..101b61a 100644
--- a/tests/storagepoolxml2xmlout/pool-scsi.xml
+++ b/tests/storagepoolxml2xmlout/pool-scsi.xml
@@ -5,7 +5,7 @@
allocation unit='bytes'0/allocation
available unit='bytes'0/available
source
-adapter type='scsi_host' name='host0'/
+adapter type='scsi_host' name='scsi_host0'/
[libvirt] [PATCH 5/8] util: Add helper to get the scsi host name by iterating over sysfs
The helper iterates over sysfs, to find out the matched scsi host
name by comparing the wwnn,wwpn pair. It's used for checkPool and
refreshPool of storage scsi backend. New helper getAdapterName
is introduced in storage_backend_scsi.c, it uses the new util
helper virGetFCHostNameByWWN to get the fc_host adapter name.
---
src/libvirt_private.syms |1 +
src/storage/storage_backend_scsi.c | 50 ++---
src/util/virutil.c | 109
src/util/virutil.h |5 ++
4 files changed, 157 insertions(+), 8 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 96f56cd..0bec5a0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1833,6 +1833,7 @@ virFindFileInPath;
virFormatIntDecimal;
virGetDeviceID;
virGetDeviceUnprivSGIO;
+virGetFCHostNameByWWN;
virGetGroupID;
virGetGroupName;
virGetHostname;
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 257001c..8166311 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -601,7 +601,8 @@ getHostNumber(const char *adapter_name)
* back-compact reason.
*/
if ((sscanf(adapter_name, scsi_host%d, host) != 1)
-(sscanf(adapter_name, host%d, host) != 1)) {
+(sscanf(adapter_name, host%d, host) != 1)
+(sscanf(adapter_name, fc_host%d, host) != 1)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to get host number from '%s'),
adapter_name);
@@ -611,42 +612,74 @@ getHostNumber(const char *adapter_name)
return host;
}
+static char *
+getAdapterName(virStoragePoolSourceAdapter adapter)
+{
+char *name = NULL;
+
+if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
+return strdup(adapter.data.name);
+
+if (!(name = virGetFCHostNameByWWN(NULL,
+adapter.data.fchost.wwnn,
+adapter.data.fchost.wwpn))) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(Failed to find SCSI host with wwnn='%s',
+ wwpn='%s'), adapter.data.fchost.wwnn,
+ adapter.data.fchost.wwpn);
+return NULL;
+}
+
+return name;
+}
+
static int
virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
bool *isActive)
{
-char *path;
+char *path = NULL;
+char *name = NULL;
int host;
+int ret = -1;
*isActive = false;
-if ((host = getHostNumber(pool-def-source.adapter.data.name)) 0)
+if (!(name = getAdapterName(pool-def-source.adapter)))
return -1;
+if ((host = getHostNumber(name)) 0)
+goto cleanup;
+
if (virAsprintf(path, /sys/class/scsi_host/host%d, host) 0) {
virReportOOMError();
-return -1;
+goto cleanup;
}
if (access(path, F_OK) == 0)
*isActive = true;
+ret = 0;
+cleanup:
VIR_FREE(path);
-
-return 0;
+VIR_FREE(name);
+return ret;
}
static int
virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool)
{
-int ret = -1;
+char *name = NULL;
int host;
+int ret = -1;
pool-def-allocation = pool-def-capacity = pool-def-available = 0;
-if ((host = getHostNumber(pool-def-source.adapter.data.name)) 0)
+if (!(name = getAdapterName(pool-def-source.adapter)))
+return -1;
+
+if ((host = getHostNumber(name)) 0)
goto out;
VIR_DEBUG(Scanning host%u, host);
@@ -658,6 +691,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
ret = 0;
out:
+VIR_FREE(name);
return ret;
}
diff --git a/src/util/virutil.c b/src/util/virutil.c
index b1d915c..4171004 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -26,6 +26,7 @@
#include config.h
+#include dirent.h
#include stdio.h
#include stdarg.h
#include stdlib.h
@@ -3447,6 +3448,105 @@ cleanup:
VIR_FREE(operation_path);
return ret;
}
+
+/* virGetHostNameByWWN:
+ *
+ * Iterate over the sysfs tree to get SCSI host name (e.g. scsi_host5)
+ * by wwnn,wwpn pair.
+ */
+char *
+virGetFCHostNameByWWN(const char *sysfs_prefix,
+ const char *wwnn,
+ const char *wwpn)
+{
+const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH;
+struct dirent *entry = NULL;
+DIR *dir = NULL;
+char *wwnn_path = NULL;
+char *wwpn_path = NULL;
+char *wwnn_buf = NULL;
+char *wwpn_buf = NULL;
+char *p;
+char *ret = NULL;
+
+if (!(dir = opendir(prefix))) {
+virReportSystemError(errno,
+ _(Failed to opendir path '%s'),
+
[libvirt] [PATCH 3/8] storage: Move virStorageBackendSCSIGetHostNumber into iscsi backend
It's only used by iscsi backend.
---
src/storage/storage_backend_iscsi.c | 39 ++-
src/storage/storage_backend_scsi.c | 39 ---
src/storage/storage_backend_scsi.h |3 --
3 files changed, 38 insertions(+), 43 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index f374961..da4367c 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -23,6 +23,7 @@
#include config.h
+#include dirent.h
#include sys/socket.h
#include netdb.h
#include sys/types.h
@@ -401,6 +402,42 @@ cleanup:
return ret;
}
+static int
+virStorageBackendISCSIGetHostNumber(const char *sysfs_path,
+ uint32_t *host)
+{
+int retval = 0;
+DIR *sysdir = NULL;
+struct dirent *dirent = NULL;
+
+VIR_DEBUG(Finding host number from '%s', sysfs_path);
+
+virFileWaitForDevices();
+
+sysdir = opendir(sysfs_path);
+
+if (sysdir == NULL) {
+virReportSystemError(errno,
+ _(Failed to opendir path '%s'), sysfs_path);
+retval = -1;
+goto out;
+}
+
+while ((dirent = readdir(sysdir))) {
+if (STREQLEN(dirent-d_name, target, strlen(target))) {
+if (sscanf(dirent-d_name,
+ target%u:, host) != 1) {
+VIR_DEBUG(Failed to parse target '%s', dirent-d_name);
+retval = -1;
+break;
+}
+}
+}
+
+closedir(sysdir);
+out:
+return retval;
+}
static int
virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
@@ -416,7 +453,7 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
return -1;
}
-if (virStorageBackendSCSIGetHostNumber(sysfs_path, host) 0) {
+if (virStorageBackendISCSIGetHostNumber(sysfs_path, host) 0) {
virReportSystemError(errno,
_(Failed to get host number for iSCSI session
with path '%s'),
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index a26bf59..257001c 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -547,45 +547,6 @@ out:
return retval;
}
-
-int
-virStorageBackendSCSIGetHostNumber(const char *sysfs_path,
- uint32_t *host)
-{
-int retval = 0;
-DIR *sysdir = NULL;
-struct dirent *dirent = NULL;
-
-VIR_DEBUG(Finding host number from '%s', sysfs_path);
-
-virFileWaitForDevices();
-
-sysdir = opendir(sysfs_path);
-
-if (sysdir == NULL) {
-virReportSystemError(errno,
- _(Failed to opendir path '%s'), sysfs_path);
-retval = -1;
-goto out;
-}
-
-while ((dirent = readdir(sysdir))) {
-if (STREQLEN(dirent-d_name, target, strlen(target))) {
-if (sscanf(dirent-d_name,
- target%u:, host) != 1) {
-VIR_DEBUG(Failed to parse target '%s', dirent-d_name);
-retval = -1;
-break;
-}
-}
-}
-
-closedir(sysdir);
-out:
-return retval;
-}
-
-
static int
virStorageBackendSCSITriggerRescan(uint32_t host)
{
diff --git a/src/storage/storage_backend_scsi.h
b/src/storage/storage_backend_scsi.h
index 1cdd0da..0984fd5 100644
--- a/src/storage/storage_backend_scsi.h
+++ b/src/storage/storage_backend_scsi.h
@@ -33,9 +33,6 @@
extern virStorageBackend virStorageBackendSCSI;
int
-virStorageBackendSCSIGetHostNumber(const char *sysfs_path,
- uint32_t *host);
-int
virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool,
uint32_t scanhost);
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] rpc: Check and message setsockopt()
Check status when attempting to set SO_REUSEADDR flag on outgoing connection
On failure, VIR_WARN(), but continue to connect. This code path is on the
sender side where the setting is just a hint and would only take effect if
the sender is overflowed with TCP connections. Inability to set doesn't mean
failure to establish a connection.
---
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 c31d383..aa523c0 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -472,7 +472,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
[libvirt] [PATCH v2 0/4] Resolved CHECKED_RETURN errors found by Coverity
Rather then further muddy the waters of the previous patch series, this is a fresh patch series. I added one patch to this since the code was in the same general area as the virBufferTrim patch. Differences in v2 rpc: Updated the commit message to include why only putting out a VIR_WARN. The code did not change. The original patch: https://www.redhat.com/archives/libvir-list/2013-January/msg00103.html tools: Rework the patch to check the virBufferTrim() status return. Added virBufferError() status check after virBufferAddLit() to match the similar check after virBufferAddChar(). The original patch: https://www.redhat.com/archives/libvir-list/2013-January/msg00118.html xen: Resend of the v2 patch to keep in the same series: https://www.redhat.com/archives/libvir-list/2013-January/msg00473.html which was the replacement for: https://www.redhat.com/archives/libvir-list/2013-January/msg00114.html util: (new) The coverity error was NEGATIVE_RETURNS. The complaint being the potential to call memset() with a negative indent value. Since virBufferGetIndent() will check and fail on buf-error, I removed that check from virBufferAdd() and used the -1 return as the way to exit. John Ferlan (4): rpc: Check and message setsockopt() tools: Check return status on virBufferTrim() xen: Ignore return status for TCP_NODELAY util: Check for negative indent in virBufferAdd src/rpc/virnetsocket.c | 4 +++- src/util/virbuffer.c| 5 ++--- src/xen/xend_internal.c | 7 +++ tools/virsh.c | 11 --- 4 files changed, 16 insertions(+), 11 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] xen: Ignore return status for TCP_NODELAY
---
src/xen/xend_internal.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 959225c..038dd1e 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -89,11 +89,10 @@ do_connect(virConnectPtr xend)
}
/*
- * try to desactivate slow-start
+ * try to deactivate slow-start
*/
-setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start,
- sizeof(no_slow_start));
-
+ignore_value(setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void
*)no_slow_start,
+sizeof(no_slow_start)));
if (connect(s, (struct sockaddr *)priv-addr, priv-addrlen) == -1) {
VIR_FORCE_CLOSE(s); /* preserves errno */
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/4] tools: Check return status on virBufferTrim()
---
tools/virsh.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index eadc519..669c9c9 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -536,6 +536,8 @@ vshTreePrintInternal(vshControl *ctl,
/* Finally print all children */
virBufferAddLit(indent, );
+if (virBufferError(indent))
+goto cleanup;
for (i = 0 ; i num_devices ; i++) {
const char *parent = (lookup)(i, true, opaque);
@@ -545,15 +547,18 @@ vshTreePrintInternal(vshControl *ctl,
false, indent) 0)
goto cleanup;
}
-virBufferTrim(indent, , -1);
+if (virBufferTrim(indent, , -1) 0)
+goto cleanup;
/* If there was no child device, and we're the last in
* a list of devices, then print another blank line */
if (nextlastdev == -1 devid == lastdev)
vshPrint(ctl, %s\n, virBufferCurrentContent(indent));
-if (!root)
-virBufferTrim(indent, NULL, 2);
+if (!root) {
+if (virBufferTrim(indent, NULL, 2) 0)
+goto cleanup;
+}
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 v2 4/4] util: Check for negative indent in virBufferAdd
--- src/util/virbuffer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c index 969dcbf..693e4b2 100644 --- a/src/util/virbuffer.c +++ b/src/util/virbuffer.c @@ -153,10 +153,9 @@ virBufferAdd(virBufferPtr buf, const char *str, int len) if (!str || !buf || (len == 0 buf-indent == 0)) return; -if (buf-error) -return; - indent = virBufferGetIndent(buf, true); +if (indent 0) +return; if (len 0) len = strlen(str); -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Add 'lxc-enter-namespace' command to virsh
From: Daniel P. Berrange berra...@redhat.com
Add a 'lxc-enter-namespace' command which accepts a domain name
and then a command + args to run, attached to the container
eg
virsh -c lxc:/// lxc-enter-namespace demo -- /bin/ps -auxf
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tools/Makefile.am| 1 +
tools/virsh-domain.c | 94
tools/virsh.pod | 8 +
3 files changed, 103 insertions(+)
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 878a114..8a0429c 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -133,6 +133,7 @@ virsh_LDADD =
\
$(STATIC_BINARIES) \
$(WARN_CFLAGS) \
../src/libvirt.la \
+ ../src/libvirt-lxc.la \
../src/libvirt-qemu.la \
../gnulib/lib/libgnu.la \
$(LIBXML_LIBS) \
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e91939c..1fee681 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -43,11 +43,13 @@
#include conf/domain_conf.h
#include console.h
#include viralloc.h
+#include vircommand.h
#include virutil.h
#include virfile.h
#include virjson.h
#include virkeycode.h
#include virmacaddr.h
+#include virprocess.h
#include virstring.h
#include virsh-domain-monitor.h
#include virerror.h
@@ -6771,6 +6773,97 @@ cleanup:
}
/*
+ * lxc-enter-namespace namespace
+ */
+static const vshCmdInfo info_lxc_enter_namespace[] = {
+{help, N_(LXC Guest Enter Namespace)},
+{desc, N_(Run an arbitrary lxc guest enter namespace; use at your own
risk)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_lxc_enter_namespace[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{NULL, 0, 0, NULL}
+};
+
+static bool
+cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+bool ret = false;
+const vshCmdOpt *opt = NULL;
+char **cmdargv = NULL;
+size_t ncmdargv = 0;
+pid_t pid;
+int nfdlist;
+int *fdlist;
+size_t i;
+
+dom = vshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+while ((opt = vshCommandOptArgv(cmd, opt))) {
+if (VIR_EXPAND_N(cmdargv, ncmdargv, 1) 0) {
+vshError(ctl, _(%s: %d: failed to allocate argv),
+ __FILE__, __LINE__);
+}
+cmdargv[ncmdargv-1] = opt-data;
+}
+if (VIR_EXPAND_N(cmdargv, ncmdargv, 1) 0) {
+vshError(ctl, _(%s: %d: failed to allocate argv),
+ __FILE__, __LINE__);
+}
+cmdargv[ncmdargv - 1] = NULL;
+
+if ((nfdlist = virDomainLxcOpenNamespace(dom, fdlist, 0)) 0)
+goto cleanup;
+
+/* Fork once because we don't want to affect
+ * virsh's namespace itself
+ */
+if (virFork(pid) 0)
+goto cleanup;
+if (pid == 0) {
+if (virDomainLxcEnterNamespace(dom,
+ nfdlist,
+ fdlist,
+ NULL,
+ NULL,
+ 0) 0)
+_exit(255);
+
+/* Fork a second time because entering the
+ * pid namespace only takes effect after fork
+ */
+if (virFork(pid) 0)
+_exit(255);
+if (pid == 0) {
+execv(cmdargv[0], cmdargv);
+_exit(255);
+} else {
+if (virProcessWait(pid, NULL) 0)
+_exit(255);
+}
+_exit(0);
+} else {
+for (i = 0 ; i nfdlist ; i++)
+VIR_FORCE_CLOSE(fdlist[i]);
+VIR_FREE(fdlist);
+if (virProcessWait(pid, NULL) 0)
+goto cleanup;
+}
+
+ret = true;
+
+cleanup:
+if (dom)
+virDomainFree(dom);
+VIR_FREE(cmdargv);
+return ret;
+}
+
+/*
* dumpxml command
*/
static const vshCmdInfo info_dumpxml[] = {
@@ -8760,6 +8853,7 @@ const vshCmdDef domManagementCmds[] = {
{inject-nmi, cmdInjectNMI, opts_inject_nmi, info_inject_nmi, 0},
{send-key, cmdSendKey, opts_send_key, info_send_key, 0},
{send-process-signal, cmdSendProcessSignal, opts_send_process_signal,
info_send_process_signal, 0},
+{lxc-enter-namespace, cmdLxcEnterNamespace, opts_lxc_enter_namespace,
info_lxc_enter_namespace, 0},
{managedsave, cmdManagedSave, opts_managedsave, info_managedsave, 0},
{managedsave-remove, cmdManagedSaveRemove, opts_managedsaveremove,
info_managedsaveremove, 0},
diff --git a/tools/virsh.pod b/tools/virsh.pod
index c9ebc8f..e2a2aec 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3056,6 +3056,14 @@ When I--aysnc is
[libvirt] [PATCH 3/5] interface: Need to check ifacedef-mac not just ifacedef after strdup()
---
src/interface/interface_backend_udev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 10b8a5d..d259043 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -577,7 +577,7 @@ udevIfaceGetIfaceDef(struct udev *udev, char *name)
/* MAC address */
ifacedef-mac = strdup(udev_device_get_sysattr_value(dev, address));
-if (!ifacedef) {
+if (!ifacedef-mac) {
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 0/5] Resolve REVERSE_INULL errors found by Coverity
This patch will resolve a set of REVERSE_INULL (CWE-476): errors found by Coverity. John Ferlan (5): network: Resolve some issues around vlan copying xen: Remove extraneous checks in xenStoreGetDomainInfo() interface: Need to check ifacedef-mac not just ifacedef after strdup() xen: Need to check validity of domain-conn before deref privateData openvz: Need to check 'vm' first before dereferencing 'def' src/interface/interface_backend_udev.c | 2 +- src/network/bridge_driver.c| 16 +--- src/openvz/openvz_driver.c | 2 +- src/xen/xen_hypervisor.c | 3 ++- src/xen/xs_internal.c | 2 +- 5 files changed, 18 insertions(+), 7 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] network: Resolve some issues around vlan copying
Remove extraneous check for 'netdef' when dereferencing for vlan.nTags.
Prior code would already check if netdef was NULL.
Coverity complained about a path where the 'vlan' was potentially valid,
but a prior checks may not have allocated 'iface-data.network.actual',
so like other paths it needs to be allocated on the fly.
---
src/network/bridge_driver.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f1be954..e2b8d06 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4005,11 +4005,21 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
vlan = iface-vlan;
else if (portgroup portgroup-vlan.nTags 0)
vlan = portgroup-vlan;
-else if (netdef netdef-vlan.nTags 0)
+else if (netdef-vlan.nTags 0)
vlan = netdef-vlan;
-if (virNetDevVlanCopy(iface-data.network.actual-vlan, vlan) 0)
-goto error;
+if (vlan) {
+/* data.network.actual may be NULL here when netdef-foward.type is
+ * VIR_NETWORK_FORWARD_{NONE|NAT|ROUTE}
+ */
+if (!iface-data.network.actual
+ (VIR_ALLOC(iface-data.network.actual) 0)) {
+virReportOOMError();
+goto error;
+}
+if (virNetDevVlanCopy(iface-data.network.actual-vlan, vlan) 0)
+goto error;
+}
validate:
/* make sure that everything now specified for the device is
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] openvz: Need to check 'vm' first before dereferencing 'def'
---
src/openvz/openvz_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index c6f814e..a7cfada 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -2053,13 +2053,13 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const
char *xml,
openvzDriverLock(driver);
vm = virDomainFindByUUID(driver-domains, dom-uuid);
-vmdef = vm-def;
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN, %s,
_(no domain with matching uuid));
goto cleanup;
}
+vmdef = vm-def;
if (virStrToLong_i(vmdef-name, NULL, 10, veid) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] parallels: Remove unused JSON fetch of OS
Commit id ac1c77f0 removed the os field in parallelsDomObj that
commit id aa296e6c had added and the data is not used by the function.
---
src/parallels/parallels_driver.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 6f33080..2c26730 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -814,9 +814,6 @@ parallelsLoadDomain(parallelsConnPtr privconn,
virJSONValuePtr jobj)
if (!(pdom-home = strdup(tmp)))
goto no_memory;
-if (!(tmp = virJSONValueObjectGetString(jobj, OS)))
-goto cleanup;
-
if (!(state = virJSONValueObjectGetString(jobj, State))) {
parallelsParseError();
goto cleanup;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] virsh: Remove unused setting of 'br_node' and 'if_node'
---
tools/virsh-interface.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index cd14e89..1e047b2 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -920,7 +920,7 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
int if_xml_size;
xmlDocPtr xml_doc = NULL;
xmlXPathContextPtr ctxt = NULL;
-xmlNodePtr top_node, br_node, if_node, cur;
+xmlNodePtr top_node, if_node, cur;
/* Get a handle to the original device */
if (!(br_handle = vshCommandOptInterfaceBy(ctl, cmd, bridge,
@@ -963,12 +963,12 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd)
VIR_FREE(if_name);
/* Find the bridge node under interface. */
-if (!(br_node = virXPathNode(./bridge, ctxt))) {
+if (virXPathNode(./bridge, ctxt) == NULL) {
vshError(ctl, %s, _(No bridge node in xml document));
goto cleanup;
}
-if ((if_node = virXPathNode(./bridge/interface[2], ctxt))) {
+if (virXPathNode(./bridge/interface[2], ctxt) != NULL) {
vshError(ctl, %s, _(Multiple interfaces attached to bridge));
goto cleanup;
}
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] xen: Need to check validity of domain-conn before deref privateData
---
src/xen/xen_hypervisor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index a770f53..ded1f0a 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -3284,7 +3284,7 @@ xenHypervisorGetDomainState(virDomainPtr domain,
int *reason,
unsigned int flags)
{
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
+xenUnifiedPrivatePtr priv;
virDomainInfo info;
virCheckFlags(0, -1);
@@ -3292,6 +3292,7 @@ xenHypervisorGetDomainState(virDomainPtr domain,
if (domain-conn == NULL)
return -1;
+priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
if (priv-handle 0 || domain-id 0)
return -1;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] util: Remove the unused setting of 'res' for virHashLookup return
---
src/util/virlockspace.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 04aeebe..163404f 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -558,13 +558,12 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace,
{
int ret = -1;
char *respath = NULL;
-virLockSpaceResourcePtr res;
VIR_DEBUG(lockspace=%p resname=%s, lockspace, resname);
virMutexLock(lockspace-lock);
-if ((res = virHashLookup(lockspace-resources, resname))) {
+if (virHashLookup(lockspace-resources, resname) != NULL) {
virReportError(VIR_ERR_RESOURCE_BUSY,
_(Lockspace resource '%s' is locked),
resname);
@@ -591,13 +590,12 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace,
{
int ret = -1;
char *respath = NULL;
-virLockSpaceResourcePtr res;
VIR_DEBUG(lockspace=%p resname=%s, lockspace, resname);
virMutexLock(lockspace-lock);
-if ((res = virHashLookup(lockspace-resources, resname))) {
+if (virHashLookup(lockspace-resources, resname) != NULL) {
virReportError(VIR_ERR_RESOURCE_BUSY,
_(Lockspace resource '%s' is locked),
resname);
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] locking: Remove unnecessary setting of lockspace
In virLockSpaceProtocolDispatchNew() the returned value of lockspace from
virLockDaemonFindLockSpace() is overwritten by the virLockSpaceNew() return.
Coverity complains that it's unused.
In virLockSpaceProtocolDispatchCreateLockSpace() lockspace is also overwritten
in a similar manner resulting in the same Coverity message.
---
src/locking/lock_daemon_dispatch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/locking/lock_daemon_dispatch.c
b/src/locking/lock_daemon_dispatch.c
index 45d2cae..4c99088 100644
--- a/src/locking/lock_daemon_dispatch.c
+++ b/src/locking/lock_daemon_dispatch.c
@@ -227,7 +227,7 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server
ATTRIBUTE_UNUSED,
goto cleanup;
}
-if ((lockspace = virLockDaemonFindLockSpace(lockDaemon, args-path))) {
+if (virLockDaemonFindLockSpace(lockDaemon, args-path) != NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Lockspace for path %s already exists),
args-path);
@@ -406,7 +406,7 @@ virLockSpaceProtocolDispatchCreateLockSpace(virNetServerPtr
server ATTRIBUTE_UNU
goto cleanup;
}
-if ((lockspace = virLockDaemonFindLockSpace(lockDaemon, args-path))) {
+if (virLockDaemonFindLockSpace(lockDaemon, args-path) != NULL) {
virReportError(VIR_ERR_OPERATION_INVALID,
_(Lockspace for path %s already exists),
args-path);
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Resolve some UNUSED_VALUE errors found by Coverity
This patch resolves UNUSED_VALUE (CWE-563) errors found by Coverity. John Ferlan (4): locking: Remove unnecessary setting of lockspace util: Remove the unused setting of 'res' for virHashLookup return virsh: Remove unused setting of 'br_node' and 'if_node' parallels: Remove unused JSON fetch of OS src/locking/lock_daemon_dispatch.c | 4 ++-- src/parallels/parallels_driver.c | 3 --- src/util/virlockspace.c| 6 ++ tools/virsh-interface.c| 6 +++--- 4 files changed, 7 insertions(+), 12 deletions(-) -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] xen: Remove extraneous checks in xenStoreGetDomainInfo()
On Tue, Jan 15, 2013 at 01:35:35PM -0500, John Ferlan wrote:
The VIR_IS_CONNECTED_DOMAIN() will already check/return -1 if domain or
domain-conn are NULL.
---
src/xen/xs_internal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index 9308522..4ccab20 100644
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -382,7 +382,7 @@ xenStoreGetDomainInfo(virDomainPtr domain,
virDomainInfoPtr info)
if (!VIR_IS_CONNECTED_DOMAIN(domain))
return -1;
-if ((domain == NULL) || (domain-conn == NULL) || (info == NULL)) {
+if (info == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
return -1;
}
Actually this entire check should be removed, along with the
VIR_IS_CONNECTED_DOMAIN check. This is obsolete code, because the same thing is
now checked
in the caller (src/libvirt.c virDomainGetInfo). Likewise for similar code
in this other files in src/xen/
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] build: add new file, for lxc_protocol checking
Commit 509eb51 added lxc_protocol.x; but without the initial
checkin of src/lxc_protocol-structs, 'make check' would fail for
anyone with pdwtags installed:
make[3]: *** No rule to make target `lxc_protocol-structs', needed by
`check-protocol'. Stop.
* src/lxc_protocol-structs: New file.
---
Pushing under the build-breaker rule.
src/lxc_protocol-structs | 13 +
1 file changed, 13 insertions(+)
create mode 100644 src/lxc_protocol-structs
diff --git a/src/lxc_protocol-structs b/src/lxc_protocol-structs
new file mode 100644
index 000..eb54172
--- /dev/null
+++ b/src/lxc_protocol-structs
@@ -0,0 +1,13 @@
+/* -*- c -*- */
+struct remote_nonnull_domain {
+remote_nonnull_string name;
+remote_uuiduuid;
+intid;
+};
+struct lxc_domain_open_namespace_args {
+remote_nonnull_domain dom;
+u_int flags;
+};
+enum lxc_procedure {
+LXC_PROC_DOMAIN_OPEN_NAMESPACE = 1,
+};
--
1.8.0.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't update count of vCPUs if hot-unplug has failed
On 01/15/2013 09:18 AM, Peter Krempa wrote: After live change of cpu counts, the number of processor threads is verified. This patch makes use of this approach to check if qemu ignored the request for cpu hot-unplug and report an appropriate message. --- src/qemu/qemu_driver.c | 9 + 1 file changed, 9 insertions(+) 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 v2] Add 'lxc-enter-namespace' command to virsh
On 01/15/2013 11:17 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Add a 'lxc-enter-namespace' command which accepts a domain name
and then a command + args to run, attached to the container
eg
virsh -c lxc:/// lxc-enter-namespace demo -- /bin/ps -auxf
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tools/Makefile.am| 1 +
tools/virsh-domain.c | 94
tools/virsh.pod | 8 +
3 files changed, 103 insertions(+)
/*
+ * lxc-enter-namespace namespace
+ */
+static const vshCmdInfo info_lxc_enter_namespace[] = {
+{help, N_(LXC Guest Enter Namespace)},
+{desc, N_(Run an arbitrary lxc guest enter namespace; use at your own
risk)},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_lxc_enter_namespace[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{NULL, 0, 0, NULL}
You deleted too much - here, you still need a VSH_OT_ARGV argument for
the parser to not reject remaining arguments of the command line.
+{cmd, VSH_OT_ARGV, VSH_OFLAG_REQ, N_(namespace)},
+++ b/tools/virsh.pod
@@ -3056,6 +3056,14 @@ When I--aysnc is given, the command waits for
timeout whether success or
failed. And when I--block is given, the command waits forever with blocking
timeout.
+=item Blxc-enter-namespace Idomain -- /path/to/binary [arg1, [arg2, ...]]
+
+Enter the namespace of Idomain and execute the command C/path/to/binary
+passing the requested args. The binary path is relative to the container
+root filesystem, not the host root filesystem. The binary will inherit the
+environment variables / console visible to virsh. This command only works
+when connected to the LXC hypervisor driver.
+
ACK with that line re-added.
--
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/5] network: Resolve some issues around vlan copying
On 01/15/2013 01:35 PM, John Ferlan wrote:
Remove extraneous check for 'netdef' when dereferencing for vlan.nTags.
Prior code would already check if netdef was NULL.
Coverity complained about a path where the 'vlan' was potentially valid,
but a prior checks may not have allocated 'iface-data.network.actual',
so like other paths it needs to be allocated on the fly.
---
src/network/bridge_driver.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f1be954..e2b8d06 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4005,11 +4005,21 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
vlan = iface-vlan;
else if (portgroup portgroup-vlan.nTags 0)
vlan = portgroup-vlan;
-else if (netdef netdef-vlan.nTags 0)
+else if (netdef-vlan.nTags 0)
vlan = netdef-vlan;
Correct. If netdef was NULL, that would mean (iface-type !=
VIR_DOMAIN_NET_TYPE_NETWORK), and in that case we would have skipped
over this code down to validate:
-if (virNetDevVlanCopy(iface-data.network.actual-vlan, vlan) 0)
-goto error;
+if (vlan) {
+/* data.network.actual may be NULL here when netdef-foward.type is
+ * VIR_NETWORK_FORWARD_{NONE|NAT|ROUTE}
+ */
+if (!iface-data.network.actual
+ (VIR_ALLOC(iface-data.network.actual) 0)) {
+virReportOOMError();
+goto error;
+}
Hmm. This ends up giving us an actual with no type set, which tells me
we should have allocated this prior to now, which points out that I
added this bit of code in the wrong place. I'll send an alternate patch
momentarily.
+if (virNetDevVlanCopy(iface-data.network.actual-vlan, vlan) 0)
+goto error;
+}
validate:
/* make sure that everything now specified for the device is
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libvirt: lxc: don't mkdir when selinux is disabled
On 01/15/2013 02:26 AM, Daniel P. Berrange wrote: On Tue, Jan 15, 2013 at 10:13:36AM +0800, Gao feng wrote: On 2013/01/09 19:20, Gao feng wrote: libvirt lxc will fail to start when selinux is disabled. error: Failed to start domain noroot error: internal error guest failed to start: PATH=/bin:/sbin TERM=linux container=lxc-libvirt container_uuid=b9873916-3516-c199-8112-1592ff694a9e LIBVIRT_LXC_UUID=b9873916-3516-c199-8112-1592ff694a9e LIBVIRT_LXC_NAME=noroot /bin/sh 2013-01-09 11:04:05.384+: 1: info : libvirt version: 1.0.1 2013-01-09 11:04:05.384+: 1: error : lxcContainerMountBasicFS:546 : Failed to mkdir /sys/fs/selinux: No such file or directory 2013-01-09 11:04:05.384+: 7536: info : libvirt version: 1.0.1 2013-01-09 11:04:05.384+: 7536: error : virLXCControllerRun:1466 : error receiving signal from container: Input/output error 2013-01-09 11:04:05.404+: 7536: error : virCommandWait:2287 : internal error Child process (ip link del veth1) unexpected exit status 1: Cannot find device veth1 fix this problem by checking if selinuxfs is mounted in host before we try to create dir /sys/fs/selinux. ACK Pushed. -- 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 5/5] openvz: Need to check 'vm' first before dereferencing 'def'
On 01/15/13 19:35, John Ferlan wrote: --- src/openvz/openvz_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] interface: Need to check ifacedef-mac not just ifacedef after strdup()
On 01/15/13 19:35, John Ferlan wrote: --- src/interface/interface_backend_udev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. (And will push in a while.) Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] Resolve some UNUSED_VALUE errors found by Coverity
On 01/15/13 19:38, John Ferlan wrote: This patch resolves UNUSED_VALUE (CWE-563) errors found by Coverity. John Ferlan (4): locking: Remove unnecessary setting of lockspace util: Remove the unused setting of 'res' for virHashLookup return virsh: Remove unused setting of 'br_node' and 'if_node' parallels: Remove unused JSON fetch of OS src/locking/lock_daemon_dispatch.c | 4 ++-- src/parallels/parallels_driver.c | 3 --- src/util/virlockspace.c| 6 ++ tools/virsh-interface.c| 6 +++--- 4 files changed, 7 insertions(+), 12 deletions(-) ACK series. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add RESUME event listener to qemu monitor.
On 01/07/2013 02:25 PM, Andres Lagar-Cavilla wrote: Perform all the appropriate plumbing. When qemu/KVM VMs are paused manually through a monitor not-owned by libvirt, libvirt will think of them as paused event after they are resumed and effectively running. With this patch the discrepancy goes away. This is meant to address bug 892791. Signed-off-by: Andres Lagar-Cavilla and...@lagarcavilla.org --- src/qemu/qemu_monitor.c | 10 src/qemu/qemu_monitor.h |3 +++ src/qemu/qemu_monitor_json.c |7 ++ src/qemu/qemu_process.c | 56 ++ 4 files changed, 76 insertions(+) Thanks again for insisting on this patch; it turns out that it solved a very real problem with 'virsh block-copy --wait --pivot ...', 'virsh dump --live', and 'virsh create-snapshot-as --live --memspec ...', for upper level applications (like VDSM) that were tracing domain state solely through libvirt events rather than through querying libvirt for its current idea of domain state. See https://bugzilla.redhat.com/show_bug.cgi?id=894085 for details, and my analysis why this patch is necessary even when there is no external monitor and no use of unsupported qemu-monitor-command. It does make me wonder if we ought to audit all callers of qemuDomainStartCPUs/qemuDomainStopCPUs to be more robust if qemu were ever to change to emit events _after_ responding to 'stop' and 'cont' monitor commands, instead of the current (lucky) results that the event always appears on the monitor before the return value of the command. -- 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/5] net: use virDomainNICModelType{From|To}String functions
On 01/15/2013 03:24 AM, Guannan Ren wrote:
On 01/14/2013 10:15 PM, John Ferlan wrote:
On 01/13/2013 10:34 AM, Guannan Ren wrote:
if (def-model) {
virBufferEscapeString(buf, model type='%s'/\n,
- def-model);
-if (STREQ(def-model, virtio)
+
virDomainNICModelTypeToString(def-model));
+if ((def-model == VIR_DOMAIN_NIC_MODEL_VIRTIO)
(def-driver.virtio.name || def-driver.virtio.txmode)) {
virBufferAddLit(buf, driver);
if (def-driver.virtio.name) {
Since model can be VIR_DOMAIN_NIC_MODEL_DEFAULT (zero), is this what
you really want?
if (def-model)
virBufferEscapeString(buf, model type='%s'/\n,
virDomainNICModelTypeToString(def-model));
if def-model is VIR_DOMAIN_NIC_MODEL_DEFAULT(0),
virDomainNICModelTypeToString
will not be executed.
For input XML VIR_DOMAIN_NIC_MODEL_DEFAULT means no particular
model is specified.
in hypervisors code, if often to set it to a default value, for
qemu , it is rtl8139.
For output XML almost, if def-mode is still
VIR_DOMAIN_NIC_MODEL_DEFAULT, it will skip
printing the model attribute. But there is slightly different
between each of hypervisors.
OK - Fair enough. I'm still getting the feel of the code. My frame of
reference was going from a NULL (char*) reference to a integer != 0
reference and thinking hmm... in certain places NULL could mean
something different.
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index c604bd2..0409b0b 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2597,10 +2597,10 @@ virVMXParseEthernet(virConfPtr conf, int
controller, virDomainNetDefPtr *def)
/* Setup virDomainNetDef */
if (connectionType == NULL || STRCASEEQ(connectionType,
bridged)) {
(*def)-type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-(*def)-model = virtualDev;
+(*def)-model = virDomainNICModelTypeFromString(virtualDev);
What if virDomainNICModelTypeFromString() 0
virtualDev is guarantee to be a valid NIC model string in the codes
above, so
there is no possibility for it to return -1.
OK, but seeing as my current frame of reference is resolving Coverity
bugs where the tool will find 3 locations in a module that handle the
return and 2 that don't and then flag the 2 that don't.
Guannan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/10] xen: Ignore return status for TCP_NODELAY
On 01/08/2013 10:40 AM, John Ferlan wrote: --- src/xen/xend_internal.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 84a25e8..01d317c 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -89,11 +89,10 @@ do_connect(virConnectPtr xend) } /* - * try to desactivate slow-start + * try to deactivate slow-start */ -setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start, - sizeof(no_slow_start)); - +ignore_value(setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (void *)no_slow_start, +sizeof(no_slow_start))); I've gone ahead and pushed this one. It seems like you have quite a few outstanding patches that are pending review; it may help to post a single message in a new thread (not buried in an existing thread) with ULRs to list archives of which messages still need some attention; https://www.redhat.com/archives/libvir-list/2013-January/thread.html may be a useful starting point. -- 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/4] FreeBSD: implement virNetDevTapCreate() and virNetDevTapDelete().
On 01/13/2013 05:51 AM, Roman Bogorodskiy wrote: Why not use virNetDevSetupControl()? Unfortunately, it's declared static, so we cannot re-use it here. But there's nothing wrong with changing it to not be static, adding it to src/libvirt_private.syms, and using it, instead of copying its body. -- 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 boolean type
On 01/14/2013 01:02 AM, Claudio Bley wrote: Nonetheless, I think it still would be valuable as point 2 and 3 still hold. Just change the definition to: typedef int virBool; I'm not too fond of using the term 'bool' for anything tri-state - to me, bool implies exactly two states. _Maybe_ you could get away with a typedef for a different name (virTristate?), but at some point, 'int' is so much easier to type than whatever new typedef, that I don't think we would be buying much with this proposal. -- 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 0/4] Resolve some UNUSED_VALUE errors found by Coverity
On 01/15/13 23:02, Peter Krempa wrote: On 01/15/13 19:38, John Ferlan wrote: This patch resolves UNUSED_VALUE (CWE-563) errors found by Coverity. John Ferlan (4): locking: Remove unnecessary setting of lockspace util: Remove the unused setting of 'res' for virHashLookup return virsh: Remove unused setting of 'br_node' and 'if_node' parallels: Remove unused JSON fetch of OS src/locking/lock_daemon_dispatch.c | 4 ++-- src/parallels/parallels_driver.c | 3 --- src/util/virlockspace.c| 6 ++ tools/virsh-interface.c| 6 +++--- 4 files changed, 7 insertions(+), 12 deletions(-) ACK series. and pushed now. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] interface: Need to check ifacedef-mac not just ifacedef after strdup()
On 01/15/13 22:31, Peter Krempa wrote: On 01/15/13 19:35, John Ferlan wrote: --- src/interface/interface_backend_udev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. (And will push in a while.) Pushed now. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] xen: Need to check validity of domain-conn before deref privateData
On 01/15/2013 11:35 AM, John Ferlan wrote:
---
src/xen/xen_hypervisor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index a770f53..ded1f0a 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -3284,7 +3284,7 @@ xenHypervisorGetDomainState(virDomainPtr domain,
int *reason,
unsigned int flags)
{
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
+xenUnifiedPrivatePtr priv;
virDomainInfo info;
virCheckFlags(0, -1);
@@ -3292,6 +3292,7 @@ xenHypervisorGetDomainState(virDomainPtr domain,
if (domain-conn == NULL)
return -1;
+priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
This should be another one of those cases like Dan mentioned for 2/5
where the caller guaranteed that domain-conn is valid, and we can get
rid of the duplicate sanity checking here.
--
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 5/5] openvz: Need to check 'vm' first before dereferencing 'def'
On 01/15/13 22:34, Peter Krempa wrote: On 01/15/13 19:35, John Ferlan wrote: --- src/openvz/openvz_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. and pushed. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't update count of vCPUs if hot-unplug has failed
On 01/15/13 20:00, Eric Blake wrote: On 01/15/2013 09:18 AM, Peter Krempa wrote: After live change of cpu counts, the number of processor threads is verified. This patch makes use of this approach to check if qemu ignored the request for cpu hot-unplug and report an appropriate message. --- src/qemu/qemu_driver.c | 9 + 1 file changed, 9 insertions(+) ACK. Pushed. Thanks. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix QEMU_CAPS_NO_ACPI detection
On 12/21/2012 06:38 AM, Ján Tomko wrote: In commit c4bbaaf8, caps-arch was checked uninitialized, rendering the whole check useless. This patch moves the conditional setting of QEMU_CAPS_NO_ACPI to qemuCapsInitQMP, and removes the no longer needed exception for S390. It also clears the flag for all non-x86 archs instead of just S390 in qemuCapsInitHelp. --- src/qemu/qemu_capabilities.c | 29 - 1 files changed, 8 insertions(+), 21 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
[libvirt] [PATCH 3/5] libxl: Don't free domain death event
Callers should not free death events provided by libxl_evdisable_FOO().
---
src/libxl/libxl_driver.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 94a78e2..61ceae1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -270,7 +270,6 @@ libxlDomainObjPrivateAlloc(void)
return NULL;
libxl_ctx_alloc(priv-ctx, LIBXL_VERSION, 0, libxl_driver-logger);
-priv-deathW = NULL;
libxl_osevent_register_hooks(priv-ctx, libxl_event_callbacks, priv);
return priv;
@@ -281,10 +280,8 @@ libxlDomainObjPrivateFree(void *data)
{
libxlDomainObjPrivatePtr priv = data;
-if (priv-deathW) {
+if (priv-deathW)
libxl_evdisable_domain_death(priv-ctx, priv-deathW);
-VIR_FREE(priv-deathW);
-}
libxl_ctx_free(priv-ctx);
VIR_FREE(priv);
@@ -604,9 +601,10 @@ libxlCreateDomEvents(virDomainObjPtr vm)
return 0;
error:
-if (priv-deathW)
+if (priv-deathW) {
libxl_evdisable_domain_death(priv-ctx, priv-deathW);
-VIR_FREE(priv-deathW);
+priv-deathW = NULL;
+}
return -1;
}
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] libxl: Check for libxl_ctx_alloc failure
---
src/libxl/libxl_driver.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 61ceae1..baa05e8 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -269,7 +269,12 @@ libxlDomainObjPrivateAlloc(void)
if (VIR_ALLOC(priv) 0)
return NULL;
-libxl_ctx_alloc(priv-ctx, LIBXL_VERSION, 0, libxl_driver-logger);
+if (libxl_ctx_alloc(priv-ctx, LIBXL_VERSION, 0, libxl_driver-logger)) {
+VIR_ERROR(_(Failed libxl context initialization));
+VIR_FREE(priv);
+return NULL;
+}
+
libxl_osevent_register_hooks(priv-ctx, libxl_event_callbacks, priv);
return priv;
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] libxl: Trivial fixes and style improvements
This patchset contains some trivial bug fixes and style improvements for the libxl driver that were noticed while investigating races between the driver and libxl. Jim Fehlig (5): libxl: Use consistent style for function definitions libxl: Use consistent parameter naming scheme libxl: Don't free domain death event libxl: Check for libxl_ctx_alloc failure libxl: Fix cleanup on domain start error src/libxl/libxl_driver.c | 106 ++- 1 file changed, 58 insertions(+), 48 deletions(-) -- 1.8.0.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] libxl: Use consistent parameter naming scheme
Use consistent parameter names throughout the libxl timeout and fd
event functions.
---
src/libxl/libxl_driver.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4b8f205..94a78e2 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -100,9 +100,9 @@ static void
libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
int fd,
int vir_events,
- void *fdinfo)
+ void *fd_info)
{
-struct libxlOSEventHookFDInfo *info = fdinfo;
+struct libxlOSEventHookFDInfo *info = fd_info;
int events = 0;
if (vir_events VIR_EVENT_HANDLE_READABLE)
@@ -182,11 +182,11 @@ libxlFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
}
static void
-libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_v)
+libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
{
-struct libxlOSEventHookTimerInfo *timer_info = timer_v;
+struct libxlOSEventHookTimerInfo *info = timer_info;
-libxl_osevent_occurred_timeout(timer_info-priv-ctx, timer_info-xl_priv);
+libxl_osevent_occurred_timeout(info-priv-ctx, info-xl_priv);
}
static void
@@ -199,7 +199,7 @@ static int
libxlTimeoutRegisterEventHook(void *priv,
void **hndp,
struct timeval abs_t,
- void *for_libxl)
+ void *xl_priv)
{
struct timeval now;
struct libxlOSEventHookTimerInfo *timer_info;
@@ -221,7 +221,7 @@ libxlTimeoutRegisterEventHook(void *priv,
}
timer_info-priv = priv;
-timer_info-xl_priv = for_libxl;
+timer_info-xl_priv = xl_priv;
timer_info-id = timer_id;
*hndp = timer_info;
return 0;
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] libxl: Use consistent style for function definitions
Commit dfa1e1dd added functions whose definitions do not conform
to the style used in the libxl driver. Change these functions to
be consistent throughout the driver.
---
src/libxl/libxl_driver.c | 79 ++--
1 file changed, 43 insertions(+), 36 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a956188..4b8f205 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1,6 +1,6 @@
/*---*/
/* Copyright (C) 2006-2012 Red Hat, Inc.
- * Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
* Copyright (C) 2011 Univention GmbH.
*
* This library is free software; you can redistribute it and/or
@@ -71,15 +71,14 @@ struct libxlOSEventHookTimerInfo {
int id;
};
-
-static void libxlDomainManagedSaveLoad(void *payload,
- const void *n ATTRIBUTE_UNUSED,
- void *opaque);
-
-
static libxlDriverPrivatePtr libxl_driver = NULL;
/* Function declarations */
+static void
+libxlDomainManagedSaveLoad(void *payload,
+ const void *n ATTRIBUTE_UNUSED,
+ void *opaque);
+
static int
libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd);
@@ -97,11 +96,11 @@ libxlDriverUnlock(libxlDriverPrivatePtr driver)
virMutexUnlock(driver-lock);
}
-
-static void libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
- int fd,
- int vir_events,
- void *fdinfo)
+static void
+libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
+ int fd,
+ int vir_events,
+ void *fdinfo)
{
struct libxlOSEventHookFDInfo *info = fdinfo;
int events = 0;
@@ -118,13 +117,15 @@ static void libxlFDEventCallback(int watch
ATTRIBUTE_UNUSED,
libxl_osevent_occurred_fd(info-priv-ctx, info-xl_priv, fd, 0, events);
}
-static void libxlFreeFDInfo(void *obj)
+static void
+libxlFreeFDInfo(void *obj)
{
VIR_FREE(obj);
}
-static int libxlFDRegisterEventHook(void *priv, int fd, void **hndp,
-short events, void *xl_priv)
+static int
+libxlFDRegisterEventHook(void *priv, int fd, void **hndp,
+ short events, void *xl_priv)
{
int vir_events = VIR_EVENT_HANDLE_ERROR;
struct libxlOSEventHookFDInfo *fdinfo;
@@ -152,10 +153,11 @@ static int libxlFDRegisterEventHook(void *priv, int fd,
void **hndp,
return 0;
}
-static int libxlFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED,
- void **hndp,
- short events)
+static int
+libxlFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ void **hndp,
+ short events)
{
struct libxlOSEventHookFDInfo *fdinfo = *hndp;
int vir_events = VIR_EVENT_HANDLE_ERROR;
@@ -169,32 +171,35 @@ static int libxlFDModifyEventHook(void *priv
ATTRIBUTE_UNUSED,
return 0;
}
-static void libxlFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
- int fd ATTRIBUTE_UNUSED,
- void *hnd)
+static void
+libxlFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
+ int fd ATTRIBUTE_UNUSED,
+ void *hnd)
{
struct libxlOSEventHookFDInfo *fdinfo = hnd;
virEventRemoveHandle(fdinfo-watch);
}
-
-static void libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_v)
+static void
+libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_v)
{
struct libxlOSEventHookTimerInfo *timer_info = timer_v;
libxl_osevent_occurred_timeout(timer_info-priv-ctx, timer_info-xl_priv);
}
-static void libxlTimerInfoFree(void* obj)
+static void
+libxlTimerInfoFree(void* obj)
{
VIR_FREE(obj);
}
-static int libxlTimeoutRegisterEventHook(void *priv,
- void **hndp,
- struct timeval abs_t,
- void *for_libxl)
+static int
+libxlTimeoutRegisterEventHook(void *priv,
+ void **hndp,
+ struct timeval abs_t,
+ void *for_libxl)
{
struct timeval now;
struct libxlOSEventHookTimerInfo *timer_info;
@@ -222,9 +227,10 @@ static int libxlTimeoutRegisterEventHook(void *priv,
return 0;
}
-static int libxlTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
- void **hndp,
-
[libvirt] [PATCH 5/5] libxl: Fix cleanup on domain start error
If building the libxl domain config fails, cleanup before returning
failure.
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index baa05e8..6da0272 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -769,7 +769,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr
vm,
libxl_domain_config_init(d_config);
if (libxlBuildDomainConfig(driver, vm-def, d_config) 0)
-return -1;
+goto error;
if (libxlFreeMem(priv, d_config) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
1.8.0.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] Convert virDomainObj, qemuAgent, qemuMonitor, lxcMonitor to virObjectLockable
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
The virDomainObj, qemuAgent, qemuMonitor, lxcMonitor classes
all require a mutex, so can be switched to use virObjectLockable
---
27 files changed, 481 insertions(+), 579 deletions(-)
Big, but looks mostly mechanical.
+++ b/src/conf/domain_conf.h
@@ -1858,9 +1858,7 @@ struct _virDomainStateReason {
typedef struct _virDomainObj virDomainObj;
typedef virDomainObj *virDomainObjPtr;
struct _virDomainObj {
-virObject object;
-
-virMutex lock;
+virObjectLockable parent;
A few of these hunks form the real meat of the change, with everything
else being fallout of using the benefits of the new parent class.
@@ -733,26 +720,18 @@ qemuAgentOpen(virDomainObjPtr vm,
if (qemuAgentInitialize() 0)
return NULL;
-if (!(mon = virObjectNew(qemuAgentClass)))
+if (!(mon = virObjectLockableNew(qemuAgentClass)))
return NULL;
-if (virMutexInit(mon-lock) 0) {
-virReportSystemError(errno, %s,
- _(cannot initialize monitor mutex));
-VIR_FREE(mon);
-return NULL;
-}
+mon-fd = -1;
Yep, I can see why you had to hoist this...
if (virCondInit(mon-notify) 0) {
virReportSystemError(errno, %s,
_(cannot initialize monitor condition));
-virMutexDestroy(mon-lock);
-VIR_FREE(mon);
+virObjectUnref(mon);
return NULL;
...when you replaced ad hoc cleanup by the parent class cleanup.
}
-mon-fd = -1;
mon-vm = vm;
mon-cb = cb;
-qemuAgentLock(mon);
switch (config-type) {
case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -791,7 +770,6 @@ qemuAgentOpen(virDomainObjPtr vm,
virObjectRef(mon);
VIR_DEBUG(New mon %p fd =%d watch=%d, mon, mon-fd, mon-watch);
-qemuAgentUnlock(mon);
Question - was it really safe to remove the lock around this section of
code, considering that you were previously handing 'mon' to
virEventAddHandle() only inside the lock? That is, now that locks are
dropped, is there any possibility that the handle you just added could
fire in the window between virEventAddHandle() and virObjectRef(), such
that you end up calling virObjectFreeCallback too soon and we end up
calling virObjectRef on a stale object?
+++ b/src/qemu/qemu_domain.c
@@ -786,12 +786,12 @@ retry:
}
while (!nested !qemuDomainNestedJobAllowed(priv, job)) {
-if (virCondWaitUntil(priv-job.asyncCond, obj-lock, then) 0)
+if (virCondWaitUntil(priv-job.asyncCond, obj-parent.lock, then)
0)
Feels a bit weird accessing the parent lock in this manner; maybe a
virObjectGetLock(obj) accessor would have been easier to read. But I'm
not too concerned; this works as-is.
Assuming the dropped qemuAgentLock() was safe,
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
[libvirt] [RFC PATCH v1 7/7] tests for virCgroup.
---
tests/Makefile.am | 5 +++
tests/vircgrouptest.c | 103 ++
2 files changed, 108 insertions(+)
create mode 100644 tests/vircgrouptest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 61b0a0c..b2ccdc1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -99,6 +99,7 @@ test_programs = virshtest sockettest \
virlockspacetest \
virstringtest \
sysinfotest \
+ vircgrouptest \
$(NULL)
if WITH_GNUTLS
@@ -640,6 +641,10 @@ utiltest_SOURCES = \
utiltest.c testutils.h testutils.c
utiltest_LDADD = $(LDADDS)
+vircgrouptest_SOURCES = \
+ vircgrouptest.c testutils.h testutils.c
+vircgrouptest_LDADD = $(LDADDS)
+
if WITH_DRIVER_MODULES
virdrivermoduletest_SOURCES = \
virdrivermoduletest.c testutils.h testutils.c
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
new file mode 100644
index 000..8d0387b
--- /dev/null
+++ b/tests/vircgrouptest.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright (C) 2012 Fujitsu.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ */
+
+#include config.h
+
+#include testutils.h
+
+#include vircgroup.h
+
+static int test_cgroup(const void *data ATTRIBUTE_UNUSED)
+{
+virCgroupItemPtr cpuset;
+
+cpuset = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET,
+ test_cgroup,
+
virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_CPUSET, false));
+if (!cpuset)
+return -1;
+
+if (virCgroupSetCpusetCpus(cpuset, 1) 0) {
+virCgroupItemFree(cpuset);
+return -1;
+}
+
+virCgroupItemFree(cpuset);
+
+return 0;
+}
+
+static int test_child_cgroup(const void *data ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+virCgroupItemPtr item = NULL, item1 = NULL, item2 = NULL;
+char *cpus;
+
+item = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET,
+test_child_cgroup,
+virCgroupGetAppRoot(VIR_CGROUP_CONTROLLER_CPUSET,
false));
+if (!item)
+goto out;
+
+item1 = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, child1, item);
+item2 = virCgroupItemNew(VIR_CGROUP_CONTROLLER_CPUSET, child2, item1);
+
+if (virCgroupGetCpusetCpus(item2, cpus) 0)
+goto out;
+
+VIR_FREE(cpus);
+
+if (virCgroupSetCpusetCpus(item2, 0) 0)
+goto out;
+
+if (virCgroupGetCpusetCpus(item2, cpus) 0)
+goto out;
+
+VIR_FREE(cpus);
+
+ret = 0;
+out:
+if (item)
+virCgroupItemFree(item);
+if (item1)
+virCgroupItemFree(item1);
+if (item2)
+virCgroupItemFree(item2);
+return ret;
+}
+
+static int
+mymain(void)
+{
+int ret = 0;
+
+if (virCgroupInit() 0)
+return -1;
+
+if (virtTestRun(test_cgroup, 1, test_cgroup, NULL) 0)
+ret = -1;
+if (virtTestRun(test_child_cgroup, 1, test_child_cgroup, NULL) 0)
+ret = -1;
+
+virCgroupUninit();
+
+return ret;
+}
+
+VIRT_TEST_MAIN(mymain)
--
1.8.0.1.240.ge8a1f5a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v1 5/7] cgroup: refactor virCgroup
This patch adds a new structure, virCgroupItem, to represent
a cgroup directory(named cgroup item). cgroup directory is
created when needed and removed if no one is using it.
---
src/libvirt_private.syms | 7 +
src/util/vircgroup.c | 411 ++-
src/util/vircgroup.h | 12 ++
3 files changed, 423 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7be58ee..636c49d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -104,6 +104,12 @@ virCgroupGetMemorySoftLimit;
virCgroupGetMemoryUsage;
virCgroupGetMemSwapHardLimit;
virCgroupGetMemSwapUsage;
+virCgroupInit;
+virCgroupItemFree;
+virCgroupItemKeyPath;
+virCgroupItemNew;
+virCgroupItemPath;
+virCgroupItemType;
virCgroupKill;
virCgroupKillPainfully;
virCgroupKillRecursive;
@@ -123,6 +129,7 @@ virCgroupSetMemory;
virCgroupSetMemoryHardLimit;
virCgroupSetMemorySoftLimit;
virCgroupSetMemSwapHardLimit;
+virCgroupUninit;
# command.h
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 71d46c5..baa0af7 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -37,6 +37,7 @@
#include libgen.h
#include dirent.h
+#include virobject.h
#include internal.h
#include virutil.h
#include viralloc.h
@@ -45,6 +46,7 @@
#include virfile.h
#include virhash.h
#include virhashcode.h
+#include virthread.h
#define CGROUP_MAX_VAL 512
@@ -58,6 +60,98 @@ struct virCgroupController {
char *placement;
};
+struct _virCgroupItem {
+virObject object;
+
+char *name;
+char *path;
+
+bool created; /* the path is created or not */
+
+virCgroupItemPtr next;
+virCgroupItemPtr parent;
+virCgroupItemPtr children;
+
+struct virCgroupController *controller;
+};
+
+static virClassPtr cgroupItemClass;
+
+static void cgroupItemDispose(void *obj);
+
+static int virCgroupItemOnceInit(void)
+{
+if (!(cgroupItemClass = virClassNew(cgroupItem,
+sizeof(virCgroupItem),
+cgroupItemDispose)))
+return -1;
+
+return 0;
+}
+VIR_ONCE_GLOBAL_INIT(virCgroupItem);
+
+static struct virCgroupController
cgroupControllers[VIR_CGROUP_CONTROLLER_LAST];
+static virCgroupItemPtr rootCgroupItems[VIR_CGROUP_CONTROLLER_LAST];
+
+static virCgroupItemPtr virCgroupItemRootNew(int type);
+static int virCgroupDetectMounts(struct virCgroupController
(*controllers)[VIR_CGROUP_CONTROLLER_LAST]);
+static int virCgroupDetectPlacement(struct virCgroupController
(*controllers)[VIR_CGROUP_CONTROLLER_LAST]);
+
+int virCgroupInit(void)
+{
+int rc;
+int i;
+
+rc = virCgroupDetectMounts(cgroupControllers);
+if (rc 0) {
+VIR_ERROR(_(Failed to initialize cgroup controllers));
+return rc;
+}
+
+rc = virCgroupDetectPlacement(cgroupControllers);
+
+if (rc == 0) {
+/* Check that for every mounted controller, we found our placement */
+for (i = 0 ; i VIR_CGROUP_CONTROLLER_LAST ; i++) {
+if (!cgroupControllers[i].mountPoint)
+continue;
+
+if (!cgroupControllers[i].placement) {
+VIR_ERROR(_(Could not find placement for controller %s at
%s),
+ virCgroupControllerTypeToString(i),
+ cgroupControllers[i].placement);
+rc = -ENOENT;
+break;
+}
+
+VIR_DEBUG(Detected mount/mapping %i:%s at %s in %s, i,
+ virCgroupControllerTypeToString(i),
+ cgroupControllers[i].mountPoint,
+ cgroupControllers[i].placement);
+}
+} else {
+VIR_ERROR(_(Failed to initialize cgroup controllers));
+}
+
+for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
+rootCgroupItems[i] = virCgroupItemRootNew(i);
+}
+
+return rc;
+}
+
+void virCgroupUninit(void)
+{
+int i;
+
+for (i = 0; i VIR_CGROUP_CONTROLLER_LAST; i++) {
+if (rootCgroupItems[i]) {
+virCgroupItemFree(rootCgroupItems[i]);
+rootCgroupItems[i] = NULL;
+}
+}
+}
+
struct virCgroup {
char *path;
@@ -74,6 +168,307 @@ typedef enum {
* cpuacct and cpuset if possible. */
} virCgroupFlags;
+static virCgroupItemPtr virCgroupItemRootNew(int type)
+{
+virCgroupItemPtr rootItem = NULL;
+
+if (type = VIR_CGROUP_CONTROLLER_LAST)
+return NULL;
+
+if (!cgroupControllers[type].mountPoint)
+return NULL;
+
+if (virCgroupItemInitialize() 0)
+return NULL;
+
+if (!(rootItem = virObjectNew(cgroupItemClass)))
+return NULL;
+
+rootItem-name = strdup(/);
+rootItem-next = NULL;
+rootItem-parent = NULL;
+rootItem-children = NULL;
+rootItem-controller = cgroupControllers[type];
+rootItem-created = 1;
+
+if (virAsprintf(rootItem-path, %s%s%s,
+
[libvirt] [RFC PATCH v1 2/7] include util.h in cgroup.h
required by VIR_ENUM_DECL. --- src/util/vircgroup.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 8b6d3b2..05f2e54 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -25,6 +25,8 @@ #ifndef __VIR_CGROUP_H__ # define __VIR_CGROUP_H__ +#include virutil.h + struct virCgroup; typedef struct virCgroup *virCgroupPtr; -- 1.8.0.1.240.ge8a1f5a -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v1 4/7] refactor virCgroupDetectMounts and virCgroupDetectPlacement
---
src/util/vircgroup.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 48cba93..71d46c5 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -113,7 +113,7 @@ bool virCgroupMounted(virCgroupPtr cgroup, int controller)
* Process /proc/mounts figuring out what controllers are
* mounted and where
*/
-static int virCgroupDetectMounts(virCgroupPtr group)
+static int virCgroupDetectMounts(struct virCgroupController
(*controllers)[VIR_CGROUP_CONTROLLER_LAST])
{
int i;
FILE *mounts = NULL;
@@ -148,8 +148,8 @@ static int virCgroupDetectMounts(virCgroupPtr group)
* first entry only
*/
if (typelen == len STREQLEN(typestr, tmp, len)
-!group-controllers[i].mountPoint
-!(group-controllers[i].mountPoint =
strdup(entry.mnt_dir)))
+!(*controllers)[i].mountPoint
+!((*controllers)[i].mountPoint = strdup(entry.mnt_dir)))
goto no_memory;
tmp = next;
}
@@ -171,7 +171,7 @@ no_memory:
* sub-path the current process is assigned to. ie not
* necessarily in the root
*/
-static int virCgroupDetectPlacement(virCgroupPtr group)
+static int virCgroupDetectPlacement(struct virCgroupController
(*cgroupControllers)[VIR_CGROUP_CONTROLLER_LAST])
{
int i;
FILE *mapping = NULL;
@@ -212,7 +212,7 @@ static int virCgroupDetectPlacement(virCgroupPtr group)
len = strlen(tmp);
}
if (typelen == len STREQLEN(typestr, tmp, len)
-!(group-controllers[i].placement = strdup(STREQ(path,
/) ? : path)))
+!((*cgroupControllers)[i].placement = strdup(STREQ(path,
/) ? : path)))
goto no_memory;
tmp = next;
@@ -236,7 +236,7 @@ static int virCgroupDetect(virCgroupPtr group)
int rc;
int i;
-rc = virCgroupDetectMounts(group);
+rc = virCgroupDetectMounts(group-controllers);
if (rc 0) {
VIR_ERROR(_(Failed to detect mounts for %s), group-path);
return rc;
@@ -251,7 +251,7 @@ static int virCgroupDetect(virCgroupPtr group)
return -ENXIO;
-rc = virCgroupDetectPlacement(group);
+rc = virCgroupDetectPlacement(group-controllers);
if (rc == 0) {
/* Check that for every mounted controller, we found our placement */
--
1.8.0.1.240.ge8a1f5a
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v1 0/7] virCgroup refactor
Hi, This series is posted for early review. This series refactors virCgroup. The changes are: - virCgroupItem is associated with a cgroup directory. The directory is created only when needed, and removed if no one is using it. - Anyone using cgroups creates instances of virCgroupItem and maintains their lifetime. Please focus on patch #5, which brings the main change(virCgroupItem), and qemu part in patch #6, which shows the usage of virCgroupItem(I've not tested lxc part yet). Hu Tao (7): call virstateCleanup to do the cleanup before libvirtd exits include util.h in cgroup.h include virterror_internal.h in threads.h refactor virCgroupDetectMounts and virCgroupDetectPlacement cgroup: refactor virCgroup deploy the newly introduced virCgroupItem. tests for virCgroup. daemon/libvirtd.c |6 + src/conf/domain_audit.c | 16 +- src/conf/domain_audit.h |6 +- src/conf/domain_conf.h|5 + src/libvirt_private.syms | 17 +- src/lxc/lxc_cgroup.c | 91 ++- src/lxc/lxc_conf.h|2 +- src/lxc/lxc_driver.c | 268 +++- src/lxc/lxc_process.c | 56 +- src/qemu/qemu_cgroup.c| 287 +++-- src/qemu/qemu_cgroup.h| 17 +- src/qemu/qemu_conf.h |2 +- src/qemu/qemu_driver.c| 478 +- src/qemu/qemu_hotplug.c | 44 +- src/qemu/qemu_migration.c | 36 +- src/qemu/qemu_process.c | 29 +- src/util/vircgroup.c | 1570 - src/util/vircgroup.h | 108 ++-- src/util/virthread.h |1 + tests/Makefile.am |5 + tests/vircgrouptest.c | 103 +++ 21 files changed, 1338 insertions(+), 1809 deletions(-) create mode 100644 tests/vircgrouptest.c -- 1.8.0.1.240.ge8a1f5a -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v1 1/7] call virstateCleanup to do the cleanup before libvirtd exits
--- daemon/libvirtd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9cdf4d9..7cb99b1 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1500,5 +1500,7 @@ cleanup: daemonConfigFree(config); +virStateCleanup(); + return ret; } -- 1.8.0.1.240.ge8a1f5a -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH v1 3/7] include virterror_internal.h in threads.h
required by virSetError. --- src/util/virthread.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virthread.h b/src/util/virthread.h index b11a251..c209440 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -23,6 +23,7 @@ # define __THREADS_H_ # include internal.h +# include virerror.h typedef struct virMutex virMutex; typedef virMutex *virMutexPtr; -- 1.8.0.1.240.ge8a1f5a -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] Convert all rpc classes over to virObjectLockable
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com --- src/rpc/virkeepalive.c | 55 +--- src/rpc/virnetclient.c | 128 +++ src/rpc/virnetclientstream.c | 67 --- src/rpc/virnetsaslcontext.c | 109 ++ src/rpc/virnetserver.c | 117 ++-- src/rpc/virnetserverclient.c | 154 +++ src/rpc/virnetsocket.c | 125 --- src/rpc/virnetsshsession.c | 82 +++ src/rpc/virnettlscontext.c | 64 +++--- 9 files changed, 366 insertions(+), 535 deletions(-) Another mostly-mechanical cleanup. 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 5/7] Add a port allocator class
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote:
From: Daniel P. Berrange berra...@redhat.com
Introduce a virPortAllocator for managing TCP port allocations.
---
.gitignore | 1 +
src/Makefile.am | 1 +
src/libvirt_private.syms | 6 ++
src/util/virportallocator.c | 188 +
src/util/virportallocator.h | 40 +
tests/Makefile.am| 17 +++-
tests/virportallocatortest.c | 195
+++
7 files changed, 447 insertions(+), 1 deletion(-)
create mode 100644 src/util/virportallocator.c
create mode 100644 src/util/virportallocator.h
create mode 100644 tests/virportallocatortest.c
+
+unsigned int start;
+unsigned int end;
+};
+
+virPortAllocatorPtr virPortAllocatorNew(unsigned short start,
+unsigned short end)
+
+{
Spurious blank line. Any reason you allocate with short, but store the
values in int internally? (unsigned short in the struct should be fine)
+virPortAllocatorPtr pa;
+
+if (start = end) {
+virReportInvalidArg(start, start port %d must be less than end port
%d,
+start, end);
+return NULL;
+}
Since this error gave a message,
+
+if (virPortAllocatorInitialize() 0)
+return NULL;
+
+if (!(pa = virObjectLockableNew(virPortAllocatorClass)))
+return NULL;
does this error need to call virReportOOMError()?
+
+pa-start = start;
+pa-end = end;
+
+if (!(pa-bitmap = virBitmapNew(end-start))) {
+virObjectUnref(pa);
+return NULL;
Same question here. Callers can't tell if a NULL return means OOM or
usage error.
+++ b/tests/Makefile.am
@@ -97,6 +97,7 @@ test_programs = virshtest sockettest \
virbitmaptest \
virlockspacetest \
virstringtest \
+virportallocatortest \
Space vs. tab issue (one of the few files where we prefer tab).
+
+VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir
/.libs/libvirportallocatormock.so)
Nice way to fake out bind() - this PRELOAD test idiom is turning out to
be useful.
ACK if you address the points above.
--
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 7/7] Convert libxl driver over to use virPortAllocator APIs
On 01/11/2013 05:13 AM, Daniel P. Berrange wrote: From: Daniel P. Berrange berra...@redhat.com Replace the current libxl driver code for managing port reservations with the new virPortAllocator APIs. --- src/libxl/libxl_conf.c | 62 src/libxl/libxl_conf.h | 4 ++-- src/libxl/libxl_driver.c | 13 +- 3 files changed, 14 insertions(+), 65 deletions(-) ACK, and nice that the two drivers now share a common resource (host port allocation) without stomping on one another. -- 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
