[libvirt] [PATCH 3/3] util: match phys_port_id when converting PF-netdev to/from VF-netdev
This patch updates functions in netdev.c to pay attention to
phys_port_id. It uses the new function virNetDevGetPhysPortID() to
learn the phys_port_id of a VF or PF, then sends that info to
virPCIGetNetName(), which has newly been modified to take an optional
phys_port_id.
---
src/util/virnetdev.c | 33 -
1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 1c150b7d7..83b255219 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1231,13 +1231,17 @@ virNetDevGetVirtualFunctions(const char *pfname,
char *pf_sysfs_device_link = NULL;
char *pci_sysfs_device_link = NULL;
char *pciConfigAddr = NULL;
+char *pfPhysPortID = NULL;
*virt_fns = NULL;
*n_vfname = 0;
*max_vfs = 0;
+if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0)
+goto cleanup;
+
if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0)
-return ret;
+goto cleanup;
if (virPCIGetVirtualFunctions(pf_sysfs_device_link, virt_fns,
n_vfname, max_vfs) < 0)
@@ -1262,8 +1266,10 @@ virNetDevGetVirtualFunctions(const char *pfname,
goto cleanup;
}
-if (virPCIGetNetName(pci_sysfs_device_link, NULL, &((*vfname)[i])) < 0)
+if (virPCIGetNetName(pci_sysfs_device_link,
+ pfPhysPortID, &((*vfname)[i])) < 0) {
goto cleanup;
+}
if (!(*vfname)[i])
VIR_INFO("VF does not have an interface name");
@@ -1276,6 +1282,7 @@ virNetDevGetVirtualFunctions(const char *pfname,
VIR_FREE(*vfname);
VIR_FREE(*virt_fns);
}
+VIR_FREE(pfPhysPortID);
VIR_FREE(pf_sysfs_device_link);
VIR_FREE(pci_sysfs_device_link);
VIR_FREE(pciConfigAddr);
@@ -1357,13 +1364,19 @@ int
virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
{
char *physfn_sysfs_path = NULL;
+char *vfPhysPortID = NULL;
int ret = -1;
+if (virNetDevGetPhysPortID(ifname, &vfPhysPortID) < 0)
+goto cleanup;
+
if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0)
-return ret;
+goto cleanup;
-if (virPCIGetNetName(physfn_sysfs_path, NULL, pfname) < 0)
+if (virPCIGetNetName(physfn_sysfs_path,
+ vfPhysPortID, pfname) < 0) {
goto cleanup;
+}
if (!*pfname) {
/* this shouldn't be possible. A VF can't exist unless its
@@ -1377,6 +1390,7 @@ virNetDevGetPhysicalFunction(const char *ifname, char
**pfname)
ret = 0;
cleanup:
+VIR_FREE(vfPhysPortID);
VIR_FREE(physfn_sysfs_path);
return ret;
}
@@ -1404,8 +1418,16 @@ virNetDevPFGetVF(const char *pfname, int vf, char
**vfname)
{
char *virtfnName = NULL;
char *virtfnSysfsPath = NULL;
+char *pfPhysPortID = NULL;
int ret = -1;
+/* a VF may have multiple "ports", each one having its own netdev,
+ * and each netdev having a different phys_port_id. Be sure we get
+ * the VF netdev with a phys_port_id matchine that of pfname
+ */
+if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0)
+goto cleanup;
+
if (virAsprintf(&virtfnName, "virtfn%d", vf) < 0)
goto cleanup;
@@ -1422,11 +1444,12 @@ virNetDevPFGetVF(const char *pfname, int vf, char
**vfname)
* isn't bound to a netdev driver, it won't have a netdev name,
* and vfname will be NULL).
*/
-ret = virPCIGetNetName(virtfnSysfsPath, NULL, vfname);
+ret = virPCIGetNetName(virtfnSysfsPath, pfPhysPortID, vfname);
cleanup:
VIR_FREE(virtfnName);
VIR_FREE(virtfnSysfsPath);
+VIR_FREE(pfPhysPortID);
return ret;
}
--
2.13.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] util: new function virNetDevGetPhysPortID()
On Linux each network device *can* (but not necessarily *does*) have
an attribute called phys_port_id which can be read from the file of
that name in the netdev's sysfs directory. The examples I've seen have
been a many-digit hexadecimal number (as an ASCII string).
This value can be useful when a single PCI device is associated with
multiple netdevs (e.g a dual port Mellanox SR-IOV NIC - this card has
a single PCI Physical Function (PF), and that PF has two netdevs
associated with it (the "net" subdirectory of the PF in sysfs has two
links rather than the usual single link to a netdev directory). Each
of the PF netdevs has a different phys_port_id. The Virtual Functions
(VF) are similar - the PF (a PCI device) has "n" VFs (also each of
these is a PCI device), each VF has two netdevs, and each of the VF
netdevs points back to the VF PCI device (with the "device" entry in
its sysfs directory) as well as having a phys_port_id matching the PF
netdev it is associated with.
virNetDevGetPhysPortID() simply attempts to read the phys_port_id for
the given netdev and return it to the caller. If this particular
netdev driver doesn't support phys_port_id (most don't), it returns
NULL (*not* a NULL-terminated string, but a NULL pointer) but still
counts it as a success.
---
src/libvirt_private.syms | 1 +
src/util/virnetdev.c | 51
src/util/virnetdev.h | 5 +
3 files changed, 57 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 32ac0835e..28d089396 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2113,6 +2113,7 @@ virNetDevGetMTU;
virNetDevGetName;
virNetDevGetOnline;
virNetDevGetPhysicalFunction;
+virNetDevGetPhysPortID;
virNetDevGetPromiscuous;
virNetDevGetRcvAllMulti;
virNetDevGetRcvMulti;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 90b7bee34..a2664de78 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1170,6 +1170,46 @@ virNetDevGetPCIDevice(const char *devName)
/**
+ * virNetDevGetPhysPortID:
+ *
+ * @ifname: name of a netdev
+ *
+ * @physPortID: pointer to char* that will receive @ifname's
+ * phys_port_id from sysfs (null terminated
+ * string). Could be NULL if @ifname's net driver doesn't
+ * support phys_port_id (most netdev drivers
+ * don't). Caller is responsible for freeing the string
+ * when finished.
+ *
+ * Returns 0 on success or -1 on failure.
+ */
+int
+virNetDevGetPhysPortID(const char *ifname,
+ char **physPortID)
+{
+int ret = -1;
+char *physPortIDFile = NULL;
+
+*physPortID = NULL;
+
+if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0)
+goto cleanup;
+
+/* a failure to read just means the driver doesn't support
+ * phys_port_id, so set success now and ignore the return from
+ * virFileReadAllQuiet().
+ */
+ret = 0;
+
+ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID));
+
+ cleanup:
+VIR_FREE(physPortIDFile);
+return ret;
+}
+
+
+/**
* virNetDevGetVirtualFunctions:
*
* @pfname : name of the physical function interface name
@@ -1433,6 +1473,17 @@ virNetDevGetVirtualFunctionInfo(const char *vfname, char
**pfname,
#else /* !__linux__ */
int
+virNetDevGetPhysPortID(const char *ifname ATTRIBUTE_UNUSED,
+ char **physPortID ATTRIBUTE_UNUSED)
+{
+/* this actually should never be called, and is just here to
+ * satisfy the linker.
+ */
+*physPortID = NULL;
+return 0;
+}
+
+int
virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED,
char ***vfname ATTRIBUTE_UNUSED,
virPCIDeviceAddressPtr **virt_fns
ATTRIBUTE_UNUSED,
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 51fcae544..9205c0e86 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -226,6 +226,11 @@ int virNetDevGetPhysicalFunction(const char *ifname, char
**pfname)
int virNetDevPFGetVF(const char *pfname, int vf, char **vfname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+int virNetDevGetPhysPortID(const char *ifname,
+ char **physPortID)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_RETURN_CHECK;
+
int virNetDevGetVirtualFunctions(const char *pfname,
char ***vfname,
virPCIDeviceAddressPtr **virt_fns,
--
2.13.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Properly deal with multiple netdevs per PCI device when converting VF <-> PF
The commit log of Patch 1 explains the majority of the reason for these patches. In short, they disambiguate the multiple netdevs per PCI device on the SRIOV PF and VFs of a Mellanox dual port NIC *when converting from a VF netdev to a PF netdev and vice versa*. This permits us to set the vlan tag and MAC address for the correct VF netdev (and detect the online status of the correct PF netdev for that VF) when using a VF in macvtap passthrough mode. (in other words, with these patches in place, it is possible to use *all* the VF netdevs on both ports of a dual port mellanox NIC for macvtap passthrough.) These patches *do not* solve the problem of saving/setting the mac address/vlan tag for both ports when using a dual port VF for PCI device assignment with VFIO (see my RFC email from yesterday). That is a much more complex problem. (These patches are a prerequisite to anything that might come out of that RFC though). Laine Stump (3): util: new function virNetDevGetPhysPortID() util: support matching a phys_port_id in virPCIGetNetName() util: match phys_port_id when converting PF-netdev to/from VF-netdev src/libvirt_private.syms | 1 + src/util/virhostdev.c| 2 +- src/util/virnetdev.c | 84 +--- src/util/virnetdev.h | 5 +++ src/util/virpci.c| 49 ++-- src/util/virpci.h| 4 ++- 6 files changed, 129 insertions(+), 16 deletions(-) -- 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] util: support matching a phys_port_id in virPCIGetNetName()
A single PCI device may have multiple netdevs associated with it. Each
of those netdevs will have a different phys_port_id entry in
sysfs. This patch modifies virPCIGetNetName() to allow matching the
netdev for a PCI device that has the same phys_port_id that the caller
wants.
---
src/util/virhostdev.c | 2 +-
src/util/virnetdev.c | 6 +++---
src/util/virpci.c | 49 -
src/util/virpci.h | 4 +++-
4 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 579563c3f..fcefebd07 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -326,7 +326,7 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char
**linkdev,
* type='hostdev'>, and it is only those devices that should
* end up calling this function.
*/
-if (virPCIGetNetName(sysfs_path, linkdev) < 0)
+if (virPCIGetNetName(sysfs_path, NULL, linkdev) < 0)
goto cleanup;
if (!linkdev) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index a2664de78..1c150b7d7 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1262,7 +1262,7 @@ virNetDevGetVirtualFunctions(const char *pfname,
goto cleanup;
}
-if (virPCIGetNetName(pci_sysfs_device_link, &((*vfname)[i])) < 0)
+if (virPCIGetNetName(pci_sysfs_device_link, NULL, &((*vfname)[i])) < 0)
goto cleanup;
if (!(*vfname)[i])
@@ -1362,7 +1362,7 @@ virNetDevGetPhysicalFunction(const char *ifname, char
**pfname)
if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0)
return ret;
-if (virPCIGetNetName(physfn_sysfs_path, pfname) < 0)
+if (virPCIGetNetName(physfn_sysfs_path, NULL, pfname) < 0)
goto cleanup;
if (!*pfname) {
@@ -1422,7 +1422,7 @@ virNetDevPFGetVF(const char *pfname, int vf, char
**vfname)
* isn't bound to a netdev driver, it won't have a netdev name,
* and vfname will be NULL).
*/
-ret = virPCIGetNetName(virtfnSysfsPath, vfname);
+ret = virPCIGetNetName(virtfnSysfsPath, NULL, vfname);
cleanup:
VIR_FREE(virtfnName);
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 2c1b75855..5d811ada6 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -24,6 +24,7 @@
#include
#include "virpci.h"
+#include "virnetdev.h"
#include
#include
@@ -2857,12 +2858,15 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr
addr,
* Returns the network device name of a pci device
*/
int
-virPCIGetNetName(char *device_link_sysfs_path, char **netname)
+virPCIGetNetName(char *device_link_sysfs_path,
+ char *physPortID,
+ char **netname)
{
char *pcidev_sysfs_net_path = NULL;
int ret = -1;
DIR *dir = NULL;
struct dirent *entry = NULL;
+char *thisPhysPortID = NULL;
if (virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path,
"net") == -1) {
@@ -2873,21 +2877,47 @@ virPCIGetNetName(char *device_link_sysfs_path, char
**netname)
if (virDirOpenQuiet(&dir, pcidev_sysfs_net_path) < 0) {
/* this *isn't* an error - caller needs to check for netname == NULL */
ret = 0;
-goto out;
+goto cleanup;
}
while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) {
-/* Assume a single directory entry */
-if (VIR_STRDUP(*netname, entry->d_name) > 0)
-ret = 0;
+/* if the caller sent a physPortID, compare it to the
+ * physportID of this netdev. If not, accept the first netdev
+ */
+if (physPortID) {
+if (virNetDevGetPhysPortID(entry->d_name, &thisPhysPortID) < 0)
+goto cleanup;
+
+/* if this one doesn't match, keep looking */
+if (STRNEQ_NULLABLE(physPortID, thisPhysPortID)) {
+VIR_FREE(thisPhysPortID);
+continue;
+}
+}
+if (VIR_STRDUP(*netname, entry->d_name) < 0)
+goto cleanup;
+
+ret = 0;
break;
}
+if (ret < 0) {
+if (physPortID) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find network device with "
+ "phys_port_id '%s' under PCI device at %s"),
+ physPortID, device_link_sysfs_path);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI device at %s had a net directory, "
+ "but it was empty"),
+ device_link_sysfs_path);
+}
+}
+ cleanup:
VIR_DIR_CLOSE(dir);
-
- out:
VIR_FREE(pcidev_sysfs_net_path);
-
+VIR_FREE(thisPhysPortID);
return ret;
}
@@ -2915,7 +2945,7 @@ virPCIGetVirtualFunctionInfo(const char
*vf_sysfs_device_path,
goto cleanup;
}
-if (v
Re: [libvirt] [libvirt-php PATCH v2 01/11] Move PHP version compat macros to utils.h
On Thu, 2017-08-03 at 14:34 -0400, Dawid Zamirski wrote:
> Also util now includes all the PHP headers.
> ---
> src/libvirt-php.c | 1 -
> src/libvirt-php.h | 157 +---
>
> src/util.h| 174
> ++
> 3 files changed, 175 insertions(+), 157 deletions(-)
>
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index 0e0c620..504a8f2 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -17,7 +17,6 @@
> #endif
>
> #include "libvirt-php.h"
> -#include "util.h"
> #include "vncfunc.h"
> #include "sockets.h"
>
> diff --git a/src/libvirt-php.h b/src/libvirt-php.h
> index bfc1934..aa3fbf3 100644
> --- a/src/libvirt-php.h
> +++ b/src/libvirt-php.h
> @@ -7,7 +7,6 @@
> #ifndef PHP_LIBVIRT_H
> #define PHP_LIBVIRT_H 1
>
> -
> /* Network constants */
> #define VIR_NETWORKS_ACTIVE 1
> #define VIR_NETWORKS_INACTIVE 2
> @@ -28,27 +27,6 @@
> #include "config.h"
> #endif
>
> -#ifdef COMPILE_DL_LIBVIRT
> -#undef PACKAGE_BUGREPORT
> -#undef PACKAGE_NAME
> -#undef PACKAGE_STRING
> -#undef PACKAGE_TARNAME
> -#undef PACKAGE_URL
> -#undef PACKAGE_VERSION
> -#include "php.h"
> -
> -#ifdef ZTS
> -#include "TSRM.h"
> -#endif
> -
> -#include "php_ini.h"
> -#ifdef EXTWIN
> -#include "ext/standard/info.h"
> -#else
> -#include "standard/info.h"
> -#endif
> -#endif
> -
> #ifndef VERSION
> #define VERSION "0.5.1"
> #define VERSION_MAJOR 0
> @@ -63,6 +41,7 @@
> #include
> #include
> #include
> +#include "util.h"
>
> #ifndef EXTWIN
> #include
> @@ -109,139 +88,6 @@ typedef uint64_t arch_uint;
> #define UINTx PRIx64
> #endif
>
> -#if PHP_MAJOR_VERSION >= 7
> -typedef size_t strsize_t;
> -typedef zend_resource virt_resource;
> -typedef virt_resource *virt_resource_handle;
> -
> -#define VIRT_RETURN_RESOURCE(_resource) \
> -RETVAL_RES(_resource)
> -
> -#define VIRT_REGISTER_RESOURCE(_resource, _le_resource) \
> -VIRT_RETURN_RESOURCE(zend_register_resource(_resource,
> _le_resource))
> -
> -#define VIRT_REGISTER_LIST_RESOURCE(_name) do { \
> -zval zret; \
> -ZVAL_RES(&zret, zend_register_resource(res_##_name,
> le_libvirt_##_name)); \
> -add_next_index_zval(return_value, &zret); \
> -} while(0)
> -
> -#define VIRT_RESOURCE_HANDLE(_resource) \
> -Z_RES_P(_resource)
> -
> -#define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
> -if ((_state = (_type)zend_fetch_resource(Z_RES_P(*_zval), _name,
> _le)) == NULL) { \
> -RETURN_FALSE; \
> -}
> -
> -#define VIRT_RETVAL_STRING(_str)\
> -RETVAL_STRING(_str)
> -#define VIRT_RETVAL_STRINGL(_str, _len) \
> -RETVAL_STRINGL(_str, _len)
> -#define VIRT_RETURN_STRING(_str)\
> -RETURN_STRING(_str)
> -#define VIRT_RETURN_STRINGL(_str, _len) \
> -RETURN_STRINGL(_str, _len)
> -#define VIRT_ZVAL_STRINGL(_zv, _str, _len) \
> -ZVAL_STRINGL(_zv, _str, _len)
> -#define VIRT_ADD_INDEX_STRING(_arg, _idx, _str) \
> -add_index_string(_arg, _idx, _str)
> -#define VIRT_ADD_NEXT_INDEX_STRING(_arg, _str) \
> -add_next_index_string(_arg, _str)
> -#define VIRT_ADD_ASSOC_STRING(_arg, _key, _str) \
> -add_assoc_string(_arg, _key, _str)
> -#define VIRT_ADD_ASSOC_STRING_EX(_arg, _key, _key_len, _value) \
> -add_assoc_string_ex(_arg, _key, _key_len, _value)
> -
> -#define VIRT_FOREACH(_ht, _pos, _zv) \
> -for (zend_hash_internal_pointer_reset_ex(_ht, &_pos); \
> - (_zv = zend_hash_get_current_data_ex(_ht, &_pos)) != NULL;
> \
> - zend_hash_move_forward_ex(_ht, &_pos)) \
> -
> -#define VIRT_FOREACH_END(_dummy)
> -
> -#define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \
> -do { \
> -zend_string *tmp_key_info; \
> -_info.type = zend_hash_get_current_key_ex(_ht, &tmp_key_info,
> &_idx, &_pos); \
> -_info.name = ZSTR_VAL(tmp_key_info); \
> -_info.length = ZSTR_LEN(tmp_key_info); \
> -} while(0)
> -
> -#define VIRT_ARRAY_INIT(_name) do { \
> -zval z##_name; \
> -_name = &z##_name; \
> -array_init(_name); \
> -} while(0)
> -
> -#else /* PHP_MAJOR_VERSION < 7 */
> -typedef int strsize_t;
> -typedef long zend_long;
> -typedef unsigned long zend_ulong;
> -typedef zend_rsrc_list_entry virt_resource;
> -typedef long virt_resource_handle;
> -
> -#define VIRT_RETURN_RESOURCE(_resource) \
> -RETVAL_RESOURCE((long) _resource)
> -
> -#define VIRT_REGISTER_RESOURCE(_resource, _le_resource) \
> -ZEND_REGISTER_RESOURCE(return_value, _resource, _le_resource)
> -
> -#define VIRT_REGISTER_LIST_RESOURCE(_name) do { \
> -zval *zret; \
> -ALLOC_INIT_ZVAL(zret); \
> -ZEND_REGISTER_RESOURCE(zret, res_##_name, le_libvirt_##_name); \
> -add_next_index_zval(return_value, zret); \
> -} while(0)
> -
> -#define VIRT_RESOURCE_HANDLE(_resource) \
> -Z_LVAL_P(_resource)
> -
> -#define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
> -ZEND_FETCH_RESOURCE(_state, _type, _zval, -1, _name, _le);
> -
Re: [libvirt] [PATCH v4 1/8] docs: schema: Add basic documentation for the virtual
On 07/07/2017 04:07 AM, Longpeng(Mike) wrote: > This patch documents XML elements used for support of virtual > crypto devices. > > In the devices section in the domain XML users may specify: > > Add an example too. > > to enable the crypto device for guests. > > Signed-off-by: Longpeng(Mike) Is this the "legal name" that would be used for a commit? Generally we prefer to see a more legal name rather than someone's email name. There's plenty of examples in git history. > --- > docs/formatdomain.html.in | 61 > +++ > docs/schemas/domaincommon.rng | 30 + > 2 files changed, 91 insertions(+) > For some reason I'm only seeing this patch from the series come through. Whether that's something specific to the RH email or in general, I'm not sure. Similarly for your v3 series, just the first patch came through. Since they were close together - I have to wonder if the RH email system was having one it's clogged or senior moments and the patches are still stuck in some queue somewhere. It's happened before, but usually everything gets backed up, not just one series from one submittor. I see from the archive you pinged on 7/25 looking for a review on the series, but even that didn't come through. It's very strange. Still I think you need to repost and adjust anyway. Here's some thoughts looking just at the archives though... Patches 1 & 3 have a "relationship" insomuch as as you're documenting in patch 1 before the domain_conf code exists. I think it's best to combine them. * For both, will the default of MODEL_VIRTIO and BACKEND_BUILTIN live for perpetuity? Or is it possible that at some point a "default" or "unknown" would be required? I ask only since both would be equal to zero for the enum and VIR_ALLOC means default to zero. So sometimes adding a "default" or "none" type entry ensures that something does get set and it's not some default as a result of the allocation algorithm that takes over. * When you add the XML parsing code, you should add the xml2xml tests. That means grabbing qemuxml2xmltest.c and xml from patches 7 & 8 and moving them into here. * For new functions, make sure there's 2 blank lines before and after the function... virDomainCryptoDefFree only has 1 before. * For the queues parse, use virStrToLong_uip to ensure no negative is supplied (per the rng below using positiveInteger) Patch 2 should be the last patch as news is always last. Patch 4 is going to need some merge conflict resolution. There is also now some tests/qemucapabilitiesdata/*ppc* replies/xml that exist - whether that relates here or not I'm not sure, but something that I think may have been added since you last posted... Patch 5... * There's an error message that has "faile" instead of "failed". * There's a switch for dev->data.crypto->model that uses VIR_DOMAIN_RNG_MODEL_LAST for a case. * Should the alias include the "virtio" in some way. Would it ever be reasonable for a domain to use two different types for different devices? Maybe virtio is supplied today and becomes legacy and who knows what is the new sleek thing next year, but both are allowed so you have to change the alias then. * You may way to create an accessor that prints the "obj%s" alias since it's formatted twice. It'll be useful if you support hotplug as well. * What about hotplug? You either should support or explicitly deny. I'm kind of surprised you didn't get build warnings because VIR_DOMAIN_DEVICE_CRYPTO wasn't added to qemu_driver.c and qemu_hotplug.c since the switch ((virDomainDeviceType) def->type) is there. * This is when the qemuxml2argvtest should be adjusted. Patch 6... Put the comma on the AddLit rather than the next virBufferAsprintf although since PPC and CCW are supported from the start, I'd say add them both at the same time. Although I do understand and appreciate why they're separate. Still it's not "new" functionality for CCW support, so just do it all at once. Patch 7... Tests are usually added at the time the command is adjusted. This looks merge-able with patches 3 and 5 Patch 8... Looks merge-able with patches 3 and 6 Couple more comments below... > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 36bea67..7c27ae7 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -7547,6 +7547,67 @@ qemu-kvm -net nic,model=? /dev/null > > > > +Crypto device > + > + > + The virtual crypto device is a virtual crypto accelerator > + card(provides crypto services, such as CIPHER, MAC, HASH, s/card(provides/card (provides) > + and AEAD) for virtual machines and it can be added to the > + guest via the crypto element. > + Since 3.1.0, QEMU and KVM only It'd be 3.7.0 at the earliest > + > + > + > + Example: usage of the crypto device: > + > + > + ... > +> + > +
Re: [libvirt] [RFC]Add new mdev interface for QoS
On Thu, 3 Aug 2017 20:26:14 +0800 "Gao, Ping A" wrote: > On 2017/8/3 0:58, Alex Williamson wrote: > > On Wed, 2 Aug 2017 21:16:28 +0530 > > Kirti Wankhede wrote: > > > >> On 8/2/2017 6:29 PM, Gao, Ping A wrote: > >>> On 2017/8/2 18:19, Kirti Wankhede wrote: > On 8/2/2017 3:56 AM, Alex Williamson wrote: > > On Tue, 1 Aug 2017 13:54:27 +0800 > > "Gao, Ping A" wrote: > > > >> On 2017/7/28 0:00, Gao, Ping A wrote: > >>> On 2017/7/27 0:43, Alex Williamson wrote: > [cc +libvir-list] > > On Wed, 26 Jul 2017 21:16:59 +0800 > "Gao, Ping A" wrote: > > > The vfio-mdev provide the capability to let different guest share > > the > > same physical device through mediate sharing, as result it bring a > > requirement about how to control the device sharing, we need a QoS > > related interface for mdev to management virtual device resource. > > > > E.g. In practical use, vGPUs assigned to different quests almost has > > different performance requirements, some guests may need higher > > priority > > for real time usage, some other may need more portion of the GPU > > resource to get higher 3D performance, corresponding we can define > > some > > interfaces like weight/cap for overall budget control, priority for > > single submission control. > > > > So I suggest to add some common attributes which are vendor > > agnostic in > > mdev core sysfs for QoS purpose. > I think what you're asking for is just some standardization of a QoS > attribute_group which a vendor can optionally include within the > existing mdev_parent_ops.mdev_attr_groups. The mdev core will > transparently enable this, but it really only provides the standard, > all of the support code is left for the vendor. I'm fine with that, > but of course the trouble with and sort of standardization is > arriving > at an agreed upon standard. Are there QoS knobs that are generic > across any mdev device type? Are there others that are more specific > to vGPU? Are there existing examples of this that we can steal their > specification? > >>> Yes, you are right, standardization QoS knobs are exactly what I > >>> wanted. > >>> Only when it become a part of the mdev framework and libvirt, then QoS > >>> such critical feature can be leveraged by cloud usage. HW vendor only > >>> need to focus on the implementation of the corresponding QoS algorithm > >>> in their back-end driver. > >>> > >>> Vfio-mdev framework provide the capability to share the device that > >>> lack > >>> of HW virtualization support to guests, no matter the device type, > >>> mediated sharing actually is a time sharing multiplex method, from > >>> this > >>> point of view, QoS can be take as a generic way about how to control > >>> the > >>> time assignment for virtual mdev device that occupy HW. As result we > >>> can > >>> define QoS knob generic across any device type by this way. Even if HW > >>> has build in with some kind of QoS support, I think it's not a problem > >>> for back-end driver to convert mdev standard QoS definition to their > >>> specification to reach the same performance expectation. Seems there > >>> are > >>> no examples for us to follow, we need define it from scratch. > >>> > >>> I proposal universal QoS control interfaces like below: > >>> > >>> Cap: The cap limits the maximum percentage of time a mdev device can > >>> own > >>> physical device. e.g. cap=60, means mdev device cannot take over 60% > >>> of > >>> total physical resource. > >>> > >>> Weight: The weight define proportional control of the mdev device > >>> resource between guests, it’s orthogonal with Cap, to target load > >>> balancing. E.g. if guest 1 should take double mdev device resource > >>> compare with guest 2, need set weight ratio to 2:1. > >>> > >>> Priority: The guest who has higher priority will get execution first, > >>> target to some real time usage and speeding interactive response. > >>> > >>> Above QoS interfaces cover both overall budget control and single > >>> submission control. I will sent out detail design later once get > >>> aligned. > >> Hi Alex, > >> Any comments about the interface mentioned above? > > Not really. > > > > Kirti, are there any QoS knobs that would be interesting > > for NVIDIA devices? > > > We have different types of vGPU for different QoS factors. > > When mdev devices are created, its resources are allocated irrespective > of which VM/userspace app is going to us
[libvirt] [libvirt-php PATCH v2 08/11] Split up the bindings for libvirt snapshot API
---
src/Makefile.am| 1 +
src/libvirt-php.c | 257 +
src/libvirt-php.h | 18 +---
src/libvirt-snapshot.c | 244 ++
src/libvirt-snapshot.h | 58 +++
5 files changed, 306 insertions(+), 272 deletions(-)
create mode 100644 src/libvirt-snapshot.c
create mode 100644 src/libvirt-snapshot.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 32b23cf..1b78011 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -26,6 +26,7 @@ libvirt_php_la_SOURCES = \
libvirt-node.c libvirt-node.h \
libvirt-stream.c libvirt-stream.h \
libvirt-domain.c libvirt-domain.h \
+ libvirt-snapshot.c libvirt-snapshot.h \
libvirt-network.c libvirt-network.h \
libvirt-storage.c libvirt-storage.h
libvirt_php_la_CFLAGS = \
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index a4108a6..6481a4a 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -23,6 +23,7 @@
#include "libvirt-node.h"
#include "libvirt-stream.h"
#include "libvirt-domain.h"
+#include "libvirt-snapshot.h"
#include "libvirt-network.h"
#include "libvirt-storage.h"
@@ -39,7 +40,6 @@ const char *features_binaries[] = { NULL };
/* ZEND thread safe per request globals definition */
int le_libvirt_nodedev;
-int le_libvirt_snapshot;
int le_libvirt_nwfilter;
ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_connect, 0, 0, 0)
@@ -476,13 +476,7 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE_LIBVIRT_CONNECTION
PHP_FE_LIBVIRT_STREAM
PHP_FE_LIBVIRT_DOMAIN
-/* Domain snapshot functions */
-PHP_FE(libvirt_domain_has_current_snapshot, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_domain_snapshot_create, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_domain_snapshot_get_xml, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_domain_snapshot_revert, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_domain_snapshot_delete, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_domain_snapshot_lookup_by_name,
arginfo_libvirt_domain_snapshot_lookup_by_name)
+PHP_FE_LIBVIRT_SNAPSHOT
PHP_FE_LIBVIRT_STORAGE
PHP_FE_LIBVIRT_NETWORK
PHP_FE_LIBVIRT_NODE
@@ -502,7 +496,6 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE(libvirt_nwfilter_lookup_by_uuid_string, arginfo_libvirt_conn_uuid)
PHP_FE(libvirt_nwfilter_lookup_by_uuid, arginfo_libvirt_conn_uuid)
/* List functions */
-PHP_FE(libvirt_list_domain_snapshots,arginfo_libvirt_conn_optflags)
PHP_FE(libvirt_list_nodedevs,arginfo_libvirt_conn_optcap)
PHP_FE(libvirt_list_all_nwfilters, arginfo_libvirt_conn)
PHP_FE(libvirt_list_nwfilters, arginfo_libvirt_conn)
@@ -1212,33 +1205,6 @@ static void php_libvirt_nodedev_dtor(virt_resource *rsrc
TSRMLS_DC)
}
}
-/* Destructor for snapshot resource */
-static void php_libvirt_snapshot_dtor(virt_resource *rsrc TSRMLS_DC)
-{
-php_libvirt_snapshot *snapshot = (php_libvirt_snapshot *)rsrc->ptr;
-int rv = 0;
-
-if (snapshot != NULL) {
-if (snapshot->snapshot != NULL) {
-if (!check_resource_allocation(NULL, INT_RESOURCE_SNAPSHOT,
snapshot->snapshot TSRMLS_CC)) {
-snapshot->snapshot = NULL;
-efree(snapshot);
-return;
-}
-rv = virDomainSnapshotFree(snapshot->snapshot);
-if (rv != 0) {
-DPRINTF("%s: virDomainSnapshotFree(%p) returned %d\n",
__FUNCTION__, snapshot->snapshot, rv);
-php_error_docref(NULL TSRMLS_CC, E_WARNING,
"virDomainSnapshotFree failed with %i on destructor: %s", rv,
LIBVIRT_G(last_error));
-} else {
-DPRINTF("%s: virDomainSnapshotFree(%p) completed
successfully\n", __FUNCTION__, snapshot->snapshot);
-resource_change_counter(INT_RESOURCE_SNAPSHOT,
snapshot->domain->conn->conn, snapshot->snapshot, 0 TSRMLS_CC);
-}
-snapshot->snapshot = NULL;
-}
-efree(snapshot);
-}
-}
-
/* Destructor for nwfilter resource */
static void php_libvirt_nwfilter_dtor(virt_resource *rsrc TSRMLS_DC)
{
@@ -1605,19 +1571,6 @@ PHP_MSHUTDOWN_FUNCTION(libvirt)
RETURN_FALSE;
\
} while (0)
-#define GET_SNAPSHOT_FROM_ARGS(args, ...)
\
-do {
\
-reset_error(TSRMLS_C);
\
-if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, args,
__VA_ARGS__) == FAILURE) { \
-set_error("Invalid arguments" TSRMLS_CC);
\
-RETURN_FALSE;
[libvirt] [libvirt-php PATCH v2 10/11] Split up the bindings for libvirt NWFilter API
---
src/Makefile.am| 3 +-
src/libvirt-nwfilter.c | 415 +
src/libvirt-nwfilter.h | 66
src/libvirt-php.c | 445 +
src/libvirt-php.h | 28
5 files changed, 487 insertions(+), 470 deletions(-)
create mode 100644 src/libvirt-nwfilter.c
create mode 100644 src/libvirt-nwfilter.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 30bebad..707a1e8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -29,7 +29,8 @@ libvirt_php_la_SOURCES = \
libvirt-snapshot.c libvirt-snapshot.h \
libvirt-storage.c libvirt-storage.h \
libvirt-network.c libvirt-network.h \
- libvirt-nodedev.c libvirt-nodedev.h
+ libvirt-nodedev.c libvirt-nodedev.h \
+ libvirt-nwfilter.c libvirt-nwfilter.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c
new file mode 100644
index 000..87dbb0b
--- /dev/null
+++ b/src/libvirt-nwfilter.c
@@ -0,0 +1,415 @@
+/*
+ * libvirt-nwfilter.c: The PHP bindings to libvirt NWFilter API
+ *
+ * See COPYING for the license of this software
+ */
+
+#include
+
+#include "libvirt-php.h"
+#include "libvirt-nwfilter.h"
+
+DEBUG_INIT("nwfilter");
+
+void
+php_libvirt_nwfilter_dtor(virt_resource *rsrc TSRMLS_DC)
+{
+php_libvirt_nwfilter *nwfilter = (php_libvirt_nwfilter *) rsrc->ptr;
+int rv = 0;
+
+if (nwfilter != NULL) {
+if (nwfilter->nwfilter != NULL) {
+if (!check_resource_allocation(NULL, INT_RESOURCE_NWFILTER,
nwfilter->nwfilter TSRMLS_CC)) {
+nwfilter->nwfilter = NULL;
+efree(nwfilter);
+
+return;
+}
+rv = virNWFilterFree(nwfilter->nwfilter);
+if (rv != 0) {
+DPRINTF("%s: virNWFilterFree(%p) returned %d\n", __FUNCTION__,
nwfilter->nwfilter, rv);
+php_error_docref(NULL TSRMLS_CC, E_WARNING, "virNWFilterFree
failed with %i on destructor: %s", rv, LIBVIRT_G(last_error));
+} else {
+DPRINTF("%s: virNWFilterFee(%p) completed successfully\n",
__FUNCTION__, nwfilter->nwfilter);
+resource_change_counter(INT_RESOURCE_NWFILTER,
nwfilter->conn->conn, nwfilter->nwfilter, 0 TSRMLS_CC);
+}
+nwfilter->nwfilter = NULL;
+}
+efree(nwfilter);
+}
+}
+
+/*
+ * Function name: libvirt_nwfilter_define_xml
+ * Since version: 0.5.4
+ * Description: Function is used to define a new nwfilter based on the XML
description
+ * Arguments: @res [resource]: libvirt connection resource
+ * @xml [string]: XML string definition of nwfilter to be
defined
+ * Returns: libvirt nwfilter resource of newly defined nwfilter
+ */
+PHP_FUNCTION(libvirt_nwfilter_define_xml)
+{
+php_libvirt_connection *conn = NULL;
+php_libvirt_nwfilter *res_nwfilter = NULL;
+virNWFilter *nwfilter;
+zval *zconn;
+char *xml = NULL;
+strsize_t xml_len;
+
+GET_CONNECTION_FROM_ARGS("rs", &zconn, &xml, &xml_len);
+
+if ((nwfilter = virNWFilterDefineXML(conn->conn, xml)) == NULL) {
+set_error_if_unset("Cannot define a new NWFilter" TSRMLS_CC);
+RETURN_FALSE;
+}
+
+res_nwfilter = (php_libvirt_nwfilter *)
emalloc(sizeof(php_libvirt_nwfilter));
+res_nwfilter->nwfilter = nwfilter;
+res_nwfilter->conn = conn;
+
+resource_change_counter(INT_RESOURCE_NWFILTER, conn->conn,
+res_nwfilter->nwfilter, 1 TSRMLS_CC);
+
+VIRT_REGISTER_RESOURCE(res_nwfilter, le_libvirt_nwfilter);
+}
+
+/*
+ * Function name: libvirt_nwfilter_undefine
+ * Since version: 0.5.4
+ * Description: Function is used to undefine already defined nwfilter
+ * Arguments: @res [resource]: libvirt nwfilter resource
+ * Returns: TRUE for success, FALSE on error
+ */
+PHP_FUNCTION(libvirt_nwfilter_undefine)
+{
+php_libvirt_nwfilter *nwfilter = NULL;
+zval *znwfilter;
+
+GET_NWFILTER_FROM_ARGS("r", &znwfilter);
+
+if (virNWFilterUndefine(nwfilter->nwfilter) != 0)
+RETURN_FALSE;
+
+RETURN_TRUE;
+}
+
+/*
+ * Function name: libvirt_nwfilter_get_xml_desc
+ * Since version: 0.5.4
+ * Description: Function is used to get the XML description for the
nwfilter
+ * Arguments: @res [resource]: libvirt nwfilter resource
+ * @xpath [string]: optional xPath expression string to get
just this entry, can be NULL
+ * Returns: nwfilter XML string or result of xPath expression
+ */
+PHP_FUNCTION(libvirt_nwfilter_get_xml_desc)
+{
+php_libvirt_nwfilter *nwfilter = NULL;
+zval *znwfilter;
+char *xml = NULL;
+char *xpath = NULL;
+char *tmp;
+strsize_t xpath_len;
+int retval = -1;
+
+GET_NWFILTER_FROM_ARGS("r|s", &znwfilter, &xpath, &xpath_len);
+
+if (xpath_len < 1)
+xpath = NUL
[libvirt] [libvirt-php PATCH v2 03/11] Split up the bindings for libvirt node API
---
src/Makefile.am | 3 +-
src/libvirt-connection.c | 1 +
src/libvirt-connection.h | 2 -
src/libvirt-node.c | 305
src/libvirt-node.h | 23
src/libvirt-php.c| 322 +--
src/libvirt-php.h| 6 -
src/util.h | 25
8 files changed, 358 insertions(+), 329 deletions(-)
create mode 100644 src/libvirt-node.c
create mode 100644 src/libvirt-node.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 0819dc6..ba2be62 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -22,7 +22,8 @@ libvirt_php_la_SOURCES = \
vncfunc.c vncfunc.h \
sockets.c sockets.h \
libvirt-php.c libvirt-php.h \
- libvirt-connection.c libvirt-connection.h
+ libvirt-connection.c libvirt-connection.h \
+ libvirt-node.c libvirt-node.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-connection.c b/src/libvirt-connection.c
index bcebd44..7e72333 100644
--- a/src/libvirt-connection.c
+++ b/src/libvirt-connection.c
@@ -6,6 +6,7 @@
#include
+#include "libvirt-php.h"
#include "libvirt-connection.h"
DEBUG_INIT("connection");
diff --git a/src/libvirt-connection.h b/src/libvirt-connection.h
index 2c50ec9..0cae5ec 100644
--- a/src/libvirt-connection.h
+++ b/src/libvirt-connection.h
@@ -7,8 +7,6 @@
#ifndef __LIBVIRT_CONNECTION_H__
# define __LIBVIRT_CONNECTION_H__
-# include "util.h"
-
# define PHP_LIBVIRT_CONNECTION_RES_NAME "Libvirt connection"
# define INT_RESOURCE_CONNECTION 0x01
# define CONNECT_FLAG_SOUNDHW_GET_NAMES 0x01
diff --git a/src/libvirt-node.c b/src/libvirt-node.c
new file mode 100644
index 000..e578802
--- /dev/null
+++ b/src/libvirt-node.c
@@ -0,0 +1,305 @@
+/*
+ * libvirt-node.c: The PHP bindings to libvirt connection API
+ *
+ * See COPYING for the license of this software
+ */
+
+#include
+
+#include "libvirt-php.h"
+#include "libvirt-connection.h"
+#include "libvirt-node.h"
+
+DEBUG_INIT("node");
+
+/*
+ * Function name: libvirt_node_get_info
+ * Since version: 0.4.1(-1)
+ * Description: Function is used to get the information about host node,
mainly total memory installed, total CPUs installed and model information are
useful
+ * Arguments: @conn [resource]: resource for connection
+ * Returns: array of node information or FALSE for error
+ */
+PHP_FUNCTION(libvirt_node_get_info)
+{
+virNodeInfo info;
+php_libvirt_connection *conn = NULL;
+zval *zconn;
+int retval;
+
+GET_CONNECTION_FROM_ARGS("r", &zconn);
+
+retval = virNodeGetInfo (conn->conn, &info);
+DPRINTF("%s: virNodeGetInfo returned %d\n", PHPFUNC, retval);
+if (retval == -1)
+RETURN_FALSE;
+
+array_init(return_value);
+VIRT_ADD_ASSOC_STRING(return_value, "model", info.model);
+add_assoc_long(return_value, "memory", (long)info.memory);
+add_assoc_long(return_value, "cpus", (long)info.cpus);
+add_assoc_long(return_value, "nodes", (long)info.nodes);
+add_assoc_long(return_value, "sockets", (long)info.sockets);
+add_assoc_long(return_value, "cores", (long)info.cores);
+add_assoc_long(return_value, "threads", (long)info.threads);
+add_assoc_long(return_value, "mhz", (long)info.mhz);
+}
+
+/*
+ * Function name: libvirt_node_get_cpu_stats
+ * Since version: 0.4.6
+ * Description: Function is used to get the CPU stats per nodes
+ * Arguments: @conn [resource]: resource for connection
+ * @cpunr [int]: CPU number to get information about,
defaults to VIR_NODE_CPU_STATS_ALL_CPUS to get information about all CPUs
+ * Returns: array of node CPU statistics including time (in seconds
since UNIX epoch), cpu number and total number of CPUs on node or FALSE for
error
+ */
+PHP_FUNCTION(libvirt_node_get_cpu_stats)
+{
+php_libvirt_connection *conn = NULL;
+zval *zconn;
+int cpuNum = VIR_NODE_CPU_STATS_ALL_CPUS;
+virNodeCPUStatsPtr params;
+virNodeInfo info;
+zend_long cpunr = -1;
+int nparams = 0;
+int i, j, numCpus;
+
+GET_CONNECTION_FROM_ARGS("r|l", &zconn, &cpunr);
+
+if (virNodeGetInfo(conn->conn, &info) != 0) {
+set_error("Cannot get number of CPUs" TSRMLS_CC);
+RETURN_FALSE;
+}
+
+numCpus = info.cpus;
+if (cpunr > numCpus - 1) {
+char tmp[256] = { 0 };
+snprintf(tmp, sizeof(tmp), "Invalid CPU number, valid numbers in range
0 to %d or VIR_NODE_CPU_STATS_ALL_CPUS",
+ numCpus - 1);
+set_error(tmp TSRMLS_CC);
+
+RETURN_FALSE;
+}
+
+cpuNum = (int)cpunr;
+
+if (virNodeGetCPUStats(conn->conn, cpuNum, NULL, &nparams, 0) != 0) {
+set_error("Cannot get number of CPU stats" TSRMLS_CC);
+RETURN_FALSE;
+}
+
+if (nparams == 0)
+RETURN_TRUE;
+
+DPRINTF("%s: Number of parameters got from virNodeGetCPUStats is %d\n",
[libvirt] [libvirt-php PATCH v2 07/11] Split up the bindings for libvirt storage API
---
src/Makefile.am |3 +-
src/libvirt-php.c | 1188 +
src/libvirt-php.h | 49 --
src/libvirt-storage.c | 1130 ++
src/libvirt-storage.h | 137 ++
5 files changed, 1271 insertions(+), 1236 deletions(-)
create mode 100644 src/libvirt-storage.c
create mode 100644 src/libvirt-storage.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 4ae01db..32b23cf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -26,7 +26,8 @@ libvirt_php_la_SOURCES = \
libvirt-node.c libvirt-node.h \
libvirt-stream.c libvirt-stream.h \
libvirt-domain.c libvirt-domain.h \
- libvirt-network.c libvirt-network.h
+ libvirt-network.c libvirt-network.h \
+ libvirt-storage.c libvirt-storage.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index e3e5554..a4108a6 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -24,6 +24,7 @@
#include "libvirt-stream.h"
#include "libvirt-domain.h"
#include "libvirt-network.h"
+#include "libvirt-storage.h"
DEBUG_INIT("core");
@@ -37,8 +38,6 @@ const char *features_binaries[] = { NULL };
#endif
/* ZEND thread safe per request globals definition */
-int le_libvirt_storagepool;
-int le_libvirt_volume;
int le_libvirt_nodedev;
int le_libvirt_snapshot;
int le_libvirt_nwfilter;
@@ -484,38 +483,7 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE(libvirt_domain_snapshot_revert, arginfo_libvirt_conn_optflags)
PHP_FE(libvirt_domain_snapshot_delete, arginfo_libvirt_conn_optflags)
PHP_FE(libvirt_domain_snapshot_lookup_by_name,
arginfo_libvirt_domain_snapshot_lookup_by_name)
-/* Storagepool functions */
-PHP_FE(libvirt_storagepool_lookup_by_name, arginfo_libvirt_conn_name)
-PHP_FE(libvirt_storagepool_lookup_by_volume, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_get_info, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagevolume_lookup_by_name, arginfo_libvirt_conn_name)
-PHP_FE(libvirt_storagevolume_lookup_by_path,
arginfo_libvirt_storagevolume_lookup_by_path)
-PHP_FE(libvirt_storagevolume_get_name, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagevolume_get_path, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagevolume_get_info, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagevolume_get_xml_desc,
arginfo_libvirt_storagevolume_get_xml_desc)
-PHP_FE(libvirt_storagevolume_create_xml, arginfo_libvirt_conn_xml)
-
PHP_FE(libvirt_storagevolume_create_xml_from,arginfo_libvirt_storagevolume_create_xml_from)
-PHP_FE(libvirt_storagevolume_delete, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_storagevolume_download,
arginfo_libvirt_storagevolume_download)
-PHP_FE(libvirt_storagevolume_upload,
arginfo_libvirt_storagevolume_download)
-PHP_FE(libvirt_storagevolume_resize,
arginfo_libvirt_storagevolume_resize)
-PHP_FE(libvirt_storagepool_get_uuid_string, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_get_name, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_lookup_by_uuid_string,
arginfo_libvirt_conn_uuid)
-PHP_FE(libvirt_storagepool_get_xml_desc, arginfo_libvirt_conn_xpath)
-PHP_FE(libvirt_storagepool_define_xml,
arginfo_libvirt_storagepool_define_xml)
-PHP_FE(libvirt_storagepool_undefine, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_create, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_destroy, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_is_active,arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_get_volume_count, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_refresh, arginfo_libvirt_conn_optflags)
-PHP_FE(libvirt_storagepool_set_autostart,arginfo_libvirt_conn_flags)
-PHP_FE(libvirt_storagepool_get_autostart,arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_build,arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_delete, arginfo_libvirt_conn)
-/* Network functions */
+PHP_FE_LIBVIRT_STORAGE
PHP_FE_LIBVIRT_NETWORK
PHP_FE_LIBVIRT_NODE
/* Nodedev functions */
@@ -536,10 +504,6 @@ static zend_function_entry libvirt_functions[] = {
/* List functions */
PHP_FE(libvirt_list_domain_snapshots,arginfo_libvirt_conn_optflags)
PHP_FE(libvirt_list_nodedevs,arginfo_libvirt_conn_optcap)
-PHP_FE(libvirt_list_storagepools,arginfo_libvirt_conn)
-PHP_FE(libvirt_list_active_storagepools, arginfo_libvirt_conn)
-PHP_FE(libvirt_list_inactive_storagepools, arginfo_libvirt_conn)
-PHP_FE(libvirt_storagepool_list_volumes, arginfo_libvirt_conn)
PHP_FE(libvirt_list_all_nwfilters, arginfo_libvirt_conn)
PHP_FE(libvirt_list_nwfilters,
[libvirt] [libvirt-php PATCH v2 01/11] Move PHP version compat macros to utils.h
Also util now includes all the PHP headers.
---
src/libvirt-php.c | 1 -
src/libvirt-php.h | 157 +---
src/util.h| 174 ++
3 files changed, 175 insertions(+), 157 deletions(-)
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 0e0c620..504a8f2 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -17,7 +17,6 @@
#endif
#include "libvirt-php.h"
-#include "util.h"
#include "vncfunc.h"
#include "sockets.h"
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index bfc1934..aa3fbf3 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -7,7 +7,6 @@
#ifndef PHP_LIBVIRT_H
#define PHP_LIBVIRT_H 1
-
/* Network constants */
#define VIR_NETWORKS_ACTIVE 1
#define VIR_NETWORKS_INACTIVE 2
@@ -28,27 +27,6 @@
#include "config.h"
#endif
-#ifdef COMPILE_DL_LIBVIRT
-#undef PACKAGE_BUGREPORT
-#undef PACKAGE_NAME
-#undef PACKAGE_STRING
-#undef PACKAGE_TARNAME
-#undef PACKAGE_URL
-#undef PACKAGE_VERSION
-#include "php.h"
-
-#ifdef ZTS
-#include "TSRM.h"
-#endif
-
-#include "php_ini.h"
-#ifdef EXTWIN
-#include "ext/standard/info.h"
-#else
-#include "standard/info.h"
-#endif
-#endif
-
#ifndef VERSION
#define VERSION "0.5.1"
#define VERSION_MAJOR 0
@@ -63,6 +41,7 @@
#include
#include
#include
+#include "util.h"
#ifndef EXTWIN
#include
@@ -109,139 +88,6 @@ typedef uint64_t arch_uint;
#define UINTx PRIx64
#endif
-#if PHP_MAJOR_VERSION >= 7
-typedef size_t strsize_t;
-typedef zend_resource virt_resource;
-typedef virt_resource *virt_resource_handle;
-
-#define VIRT_RETURN_RESOURCE(_resource) \
-RETVAL_RES(_resource)
-
-#define VIRT_REGISTER_RESOURCE(_resource, _le_resource) \
-VIRT_RETURN_RESOURCE(zend_register_resource(_resource, _le_resource))
-
-#define VIRT_REGISTER_LIST_RESOURCE(_name) do { \
-zval zret; \
-ZVAL_RES(&zret, zend_register_resource(res_##_name, le_libvirt_##_name)); \
-add_next_index_zval(return_value, &zret); \
-} while(0)
-
-#define VIRT_RESOURCE_HANDLE(_resource) \
-Z_RES_P(_resource)
-
-#define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
-if ((_state = (_type)zend_fetch_resource(Z_RES_P(*_zval), _name, _le)) ==
NULL) { \
-RETURN_FALSE; \
-}
-
-#define VIRT_RETVAL_STRING(_str)\
-RETVAL_STRING(_str)
-#define VIRT_RETVAL_STRINGL(_str, _len) \
-RETVAL_STRINGL(_str, _len)
-#define VIRT_RETURN_STRING(_str)\
-RETURN_STRING(_str)
-#define VIRT_RETURN_STRINGL(_str, _len) \
-RETURN_STRINGL(_str, _len)
-#define VIRT_ZVAL_STRINGL(_zv, _str, _len) \
-ZVAL_STRINGL(_zv, _str, _len)
-#define VIRT_ADD_INDEX_STRING(_arg, _idx, _str) \
-add_index_string(_arg, _idx, _str)
-#define VIRT_ADD_NEXT_INDEX_STRING(_arg, _str) \
-add_next_index_string(_arg, _str)
-#define VIRT_ADD_ASSOC_STRING(_arg, _key, _str) \
-add_assoc_string(_arg, _key, _str)
-#define VIRT_ADD_ASSOC_STRING_EX(_arg, _key, _key_len, _value) \
-add_assoc_string_ex(_arg, _key, _key_len, _value)
-
-#define VIRT_FOREACH(_ht, _pos, _zv) \
-for (zend_hash_internal_pointer_reset_ex(_ht, &_pos); \
- (_zv = zend_hash_get_current_data_ex(_ht, &_pos)) != NULL; \
- zend_hash_move_forward_ex(_ht, &_pos)) \
-
-#define VIRT_FOREACH_END(_dummy)
-
-#define VIRT_HASH_CURRENT_KEY_INFO(_ht, _pos, _idx, _info) \
-do { \
-zend_string *tmp_key_info; \
-_info.type = zend_hash_get_current_key_ex(_ht, &tmp_key_info, &_idx,
&_pos); \
-_info.name = ZSTR_VAL(tmp_key_info); \
-_info.length = ZSTR_LEN(tmp_key_info); \
-} while(0)
-
-#define VIRT_ARRAY_INIT(_name) do { \
-zval z##_name; \
-_name = &z##_name; \
-array_init(_name); \
-} while(0)
-
-#else /* PHP_MAJOR_VERSION < 7 */
-typedef int strsize_t;
-typedef long zend_long;
-typedef unsigned long zend_ulong;
-typedef zend_rsrc_list_entry virt_resource;
-typedef long virt_resource_handle;
-
-#define VIRT_RETURN_RESOURCE(_resource) \
-RETVAL_RESOURCE((long) _resource)
-
-#define VIRT_REGISTER_RESOURCE(_resource, _le_resource) \
-ZEND_REGISTER_RESOURCE(return_value, _resource, _le_resource)
-
-#define VIRT_REGISTER_LIST_RESOURCE(_name) do { \
-zval *zret; \
-ALLOC_INIT_ZVAL(zret); \
-ZEND_REGISTER_RESOURCE(zret, res_##_name, le_libvirt_##_name); \
-add_next_index_zval(return_value, zret); \
-} while(0)
-
-#define VIRT_RESOURCE_HANDLE(_resource) \
-Z_LVAL_P(_resource)
-
-#define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
-ZEND_FETCH_RESOURCE(_state, _type, _zval, -1, _name, _le);
-
-#define VIRT_RETVAL_STRING(_str)\
-RETVAL_STRING(_str, 1)
-#define VIRT_RETVAL_STRINGL(_str, _len) \
-RETVAL_STRINGL(_str, _len, 1)
-#define VIRT_RETURN_STRING(_str)\
-RETURN_STRING(_str, 1)
-#define VIRT_RETURN_STRINGL(_str, _len) \
-RETURN_STRINGL(_str, _len, 1)
-#define VIRT_ZVAL_STRINGL(_zv, _str, _len) \
-ZVAL_STRINGL(_zv, _str, _len, 1)
-#define VIRT_ADD_
[libvirt] [libvirt-php PATCH v2 06/11] Split up the bindings for libvirt network API
---
src/Makefile.am | 3 +-
src/libvirt-network.c | 587
src/libvirt-network.h | 73 ++
src/libvirt-php.c | 610 +-
src/libvirt-php.h | 24 +-
5 files changed, 665 insertions(+), 632 deletions(-)
create mode 100644 src/libvirt-network.c
create mode 100644 src/libvirt-network.h
diff --git a/src/Makefile.am b/src/Makefile.am
index b8eae3a..4ae01db 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -25,7 +25,8 @@ libvirt_php_la_SOURCES = \
libvirt-connection.c libvirt-connection.h \
libvirt-node.c libvirt-node.h \
libvirt-stream.c libvirt-stream.h \
- libvirt-domain.c libvirt-domain.h
+ libvirt-domain.c libvirt-domain.h \
+ libvirt-network.c libvirt-network.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
new file mode 100644
index 000..464b060
--- /dev/null
+++ b/src/libvirt-network.c
@@ -0,0 +1,587 @@
+/*
+ * libvirt-network.c: The PHP bindings to libvirt network API
+ *
+ * See COPYING for the license of this software
+ */
+
+#include
+
+#include "libvirt-php.h"
+#include "libvirt-network.h"
+
+DEBUG_INIT("network");
+
+void
+php_libvirt_network_dtor(virt_resource *rsrc TSRMLS_DC)
+{
+php_libvirt_network *network = (php_libvirt_network *)rsrc->ptr;
+int rv = 0;
+
+if (network != NULL) {
+if (network->network != NULL) {
+if (!check_resource_allocation(network->conn->conn,
INT_RESOURCE_NETWORK, network->network TSRMLS_CC)) {
+network->network = NULL;
+efree(network);
+return;
+}
+rv = virNetworkFree(network->network);
+if (rv != 0) {
+DPRINTF("%s: virNetworkFree(%p) returned %d (%s)\n",
__FUNCTION__, network->network, rv, LIBVIRT_G(last_error));
+php_error_docref(NULL TSRMLS_CC, E_WARNING, "virStorageVolFree
failed with %i on destructor: %s", rv, LIBVIRT_G(last_error));
+} else {
+DPRINTF("%s: virNetworkFree(%p) completed successfully\n",
__FUNCTION__, network->network);
+resource_change_counter(INT_RESOURCE_NETWORK, NULL,
network->network, 0 TSRMLS_CC);
+}
+network->network = NULL;
+}
+efree(network);
+}
+}
+
+/*
+ * Function name: libvirt_network_define_xml
+ * Since version: 0.4.2
+ * Description: Function is used to define a new virtual network based on
the XML description
+ * Arguments: @res [resource]: libvirt connection resource
+ * @xml [string]: XML string definition of network to be
defined
+ * Returns: libvirt network resource of newly defined network
+ */
+PHP_FUNCTION(libvirt_network_define_xml)
+{
+php_libvirt_connection *conn = NULL;
+php_libvirt_network *res_net = NULL;
+virNetwork *net;
+zval *zconn;
+char *xml = NULL;
+strsize_t xml_len;
+
+GET_CONNECTION_FROM_ARGS("rs", &zconn, &xml, &xml_len);
+
+if ((net = virNetworkDefineXML(conn->conn, xml)) == NULL) {
+set_error_if_unset("Cannot define a new network" TSRMLS_CC);
+RETURN_FALSE;
+}
+
+res_net = (php_libvirt_network *)emalloc(sizeof(php_libvirt_network));
+res_net->network = net;
+res_net->conn = conn;
+
+DPRINTF("%s: returning %p\n", PHPFUNC, res_net->network);
+resource_change_counter(INT_RESOURCE_NETWORK, conn->conn,
res_net->network, 1 TSRMLS_CC);
+
+VIRT_REGISTER_RESOURCE(res_net, le_libvirt_network);
+}
+
+/*
+ * Function name: libvirt_network_get_xml_desc
+ * Since version: 0.4.1(-1)
+ * Description: Function is used to get the XML description for the network
+ * Arguments: @res [resource]: libvirt network resource
+ * @xpath [string]: optional xPath expression string to get
just this entry, can be NULL
+ * Returns: network XML string or result of xPath expression
+ */
+PHP_FUNCTION(libvirt_network_get_xml_desc)
+{
+php_libvirt_network *network;
+zval *znetwork;
+char *xml = NULL;
+char *xpath = NULL;
+char *tmp;
+strsize_t xpath_len;
+int retval = -1;
+
+GET_NETWORK_FROM_ARGS("r|s", &znetwork, &xpath, &xpath_len);
+if (xpath_len < 1)
+xpath = NULL;
+
+xml = virNetworkGetXMLDesc(network->network, 0);
+
+if (xml == NULL) {
+set_error_if_unset("Cannot get network XML" TSRMLS_CC);
+RETURN_FALSE;
+}
+
+tmp = get_string_from_xpath(xml, xpath, NULL, &retval);
+if ((tmp == NULL) || (retval < 0)) {
+VIRT_RETVAL_STRING(xml);
+} else {
+VIRT_RETVAL_STRING(tmp);
+}
+
+free(xml);
+free(tmp);
+}
+
+/*
+ * Function name: libvirt_network_undefine
+ * Since version: 0.4.2
+ * Description: Function is used to undefine already defined network
+ * Arguments: @res [resource
[libvirt] [libvirt-php PATCH v2 04/11] Split up the bindings for libvirt stream API
---
src/Makefile.am | 3 +-
src/libvirt-php.c| 231 +--
src/libvirt-php.h| 15 +---
src/libvirt-stream.c | 230 ++
src/libvirt-stream.h | 39 +
5 files changed, 274 insertions(+), 244 deletions(-)
create mode 100644 src/libvirt-stream.c
create mode 100644 src/libvirt-stream.h
diff --git a/src/Makefile.am b/src/Makefile.am
index ba2be62..ddfad41 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -23,7 +23,8 @@ libvirt_php_la_SOURCES = \
sockets.c sockets.h \
libvirt-php.c libvirt-php.h \
libvirt-connection.c libvirt-connection.h \
- libvirt-node.c libvirt-node.h
+ libvirt-node.c libvirt-node.h \
+ libvirt-stream.c libvirt-stream.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 6315ec4..c84aaef 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -21,11 +21,10 @@
#include "sockets.h"
#include "libvirt-connection.h"
#include "libvirt-node.h"
+#include "libvirt-stream.h"
DEBUG_INIT("core");
-
-
#ifndef EXTWIN
/* Additional binaries */
const char *features[] = { "screenshot", "create-image", "screenshot-convert",
NULL };
@@ -41,7 +40,6 @@ int le_libvirt_storagepool;
int le_libvirt_volume;
int le_libvirt_network;
int le_libvirt_nodedev;
-int le_libvirt_stream;
int le_libvirt_snapshot;
int le_libvirt_nwfilter;
@@ -476,15 +474,8 @@ ZEND_END_ARG_INFO()
static zend_function_entry libvirt_functions[] = {
/* Common functions */
PHP_FE(libvirt_get_last_error, arginfo_libvirt_void)
-/* Connect functions */
PHP_FE_LIBVIRT_CONNECTION
-/* Stream functions */
-PHP_FE(libvirt_stream_create,arginfo_libvirt_conn)
-PHP_FE(libvirt_stream_close, arginfo_libvirt_conn)
-PHP_FE(libvirt_stream_abort, arginfo_libvirt_conn)
-PHP_FE(libvirt_stream_finish,arginfo_libvirt_conn)
-PHP_FE(libvirt_stream_send, arginfo_libvirt_stream_send)
-PHP_FE(libvirt_stream_recv, arginfo_libvirt_stream_recv)
+PHP_FE_LIBVIRT_STREAM
/* Domain functions */
PHP_FE(libvirt_domain_new, arginfo_libvirt_domain_new)
PHP_FE(libvirt_domain_new_get_vnc, arginfo_libvirt_void)
@@ -1357,33 +1348,6 @@ static void php_libvirt_domain_dtor(virt_resource *rsrc
TSRMLS_DC)
}
}
-/* Destructor for stream resource */
-static void php_libvirt_stream_dtor(virt_resource *rsrc TSRMLS_DC)
-{
-php_libvirt_stream *stream = (php_libvirt_stream *)rsrc->ptr;
-int rv = 0;
-
-if (stream != NULL) {
-if (stream->stream != NULL) {
-if (!check_resource_allocation(NULL, INT_RESOURCE_STREAM,
stream->stream TSRMLS_CC)) {
-stream->stream = NULL;
-efree(stream);
-return;
-}
-rv = virStreamFree(stream->stream);
-if (rv != 0) {
-DPRINTF("%s: virStreamFree(%p) returned %d (%s)\n",
__FUNCTION__, stream->stream, rv, LIBVIRT_G(last_error));
-php_error_docref(NULL TSRMLS_CC, E_WARNING, "virStreamFree
failed with %i on destructor: %s", rv, LIBVIRT_G(last_error));
-} else {
-DPRINTF("%s: virStreamFree(%p) completed successfully\n",
__FUNCTION__, stream->stream);
-resource_change_counter(INT_RESOURCE_STREAM, NULL,
stream->stream, 0 TSRMLS_CC);
-}
-stream->stream = NULL;
-}
-efree(stream);
-}
-}
-
/* Destructor for storagepool resource */
static void php_libvirt_storagepool_dtor(virt_resource *rsrc TSRMLS_DC)
{
@@ -3251,197 +3215,6 @@ PHP_FUNCTION(libvirt_domain_lookup_by_uuid_string)
}
/*
- * Function name: libvirt_stream_create
- * Since version: 0.5.0
- * Description: Function is used to create new stream from libvirt conn
- * Arguments: @res [resource]: libvirt connection resource from
libvirt_connect()
- * Returns: resource libvirt stream resource
- */
-PHP_FUNCTION(libvirt_stream_create)
-{
-php_libvirt_connection *conn = NULL;
-zval *zconn;
-virStreamPtr stream = NULL;
-php_libvirt_stream *res_stream;
-
-if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "r", &zconn) ==
FAILURE)
-RETURN_FALSE;
-VIRT_FETCH_RESOURCE(conn, php_libvirt_connection*, &zconn,
PHP_LIBVIRT_CONNECTION_RES_NAME, le_libvirt_connection);
-if ((conn == NULL) || (conn->conn == NULL))
-RETURN_FALSE;
-
-stream = virStreamNew(conn->conn, 0);
-if (stream == NULL) {
-set_error("Cannot create new stream" TSRMLS_CC);
-RETURN_FALSE;
-}
-
-res_stream = (php_libvirt_stream *)emalloc(sizeof(php_libvirt_stream));
-res_stream->stream = stream;
-res_stream->conn = conn;
-
-resource_change_counter(INT_R
[libvirt] [libvirt-php PATCH v2 09/11] Split up the bindings for libvirt nodedev API
---
src/Makefile.am | 3 +-
src/libvirt-nodedev.c | 340
src/libvirt-nodedev.h | 54
src/libvirt-php.c | 353 +-
src/libvirt-php.h | 13 --
5 files changed, 399 insertions(+), 364 deletions(-)
create mode 100644 src/libvirt-nodedev.c
create mode 100644 src/libvirt-nodedev.h
diff --git a/src/Makefile.am b/src/Makefile.am
index 1b78011..30bebad 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -27,8 +27,9 @@ libvirt_php_la_SOURCES = \
libvirt-stream.c libvirt-stream.h \
libvirt-domain.c libvirt-domain.h \
libvirt-snapshot.c libvirt-snapshot.h \
+ libvirt-storage.c libvirt-storage.h \
libvirt-network.c libvirt-network.h \
- libvirt-storage.c libvirt-storage.h
+ libvirt-nodedev.c libvirt-nodedev.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
new file mode 100644
index 000..03f7257
--- /dev/null
+++ b/src/libvirt-nodedev.c
@@ -0,0 +1,340 @@
+/*
+ * libvirt-nodedev.c: The PHP bindings to libvirt nodedev API
+ *
+ * See COPYING for the license of this software
+ */
+
+#include
+
+#include "libvirt-php.h"
+#include "libvirt-nodedev.h"
+
+DEBUG_INIT("nodedev");
+
+void
+php_libvirt_nodedev_dtor(virt_resource *rsrc TSRMLS_DC)
+{
+php_libvirt_nodedev *nodedev = (php_libvirt_nodedev *)rsrc->ptr;
+int rv = 0;
+
+if (nodedev != NULL) {
+if (nodedev->device != NULL) {
+if (!check_resource_allocation(nodedev->conn->conn,
INT_RESOURCE_NODEDEV, nodedev->device TSRMLS_CC)) {
+nodedev->device = NULL;
+efree(nodedev);
+return;
+}
+rv = virNodeDeviceFree(nodedev->device);
+if (rv != 0) {
+DPRINTF("%s: virNodeDeviceFree(%p) returned %d (%s)\n",
__FUNCTION__, nodedev->device, rv, LIBVIRT_G(last_error));
+php_error_docref(NULL TSRMLS_CC, E_WARNING, "virStorageVolFree
failed with %i on destructor: %s", rv, LIBVIRT_G(last_error));
+} else {
+DPRINTF("%s: virNodeDeviceFree(%p) completed successfully\n",
__FUNCTION__, nodedev->device);
+resource_change_counter(INT_RESOURCE_NODEDEV,
nodedev->conn->conn, nodedev->device, 0 TSRMLS_CC);
+}
+nodedev->device = NULL;
+}
+efree(nodedev);
+}
+}
+
+/*
+ * Function name: libvirt_nodedev_get
+ * Since version: 0.4.1(-1)
+ * Description: Function is used to get the node device by it's name
+ * Arguments: @res [resource]: libvirt connection resource
+ * @name [string]: name of the nodedev to get resource
+ * Returns: libvirt nodedev resource
+ */
+PHP_FUNCTION(libvirt_nodedev_get)
+{
+php_libvirt_connection *conn = NULL;
+php_libvirt_nodedev *res_dev = NULL;
+virNodeDevice *dev;
+zval *zconn;
+char *name;
+strsize_t name_len;
+
+GET_CONNECTION_FROM_ARGS("rs", &zconn, &name, &name_len);
+
+if ((dev = virNodeDeviceLookupByName(conn->conn, name)) == NULL) {
+set_error("Cannot get find requested node device" TSRMLS_CC);
+RETURN_FALSE;
+}
+
+res_dev = (php_libvirt_nodedev *)emalloc(sizeof(php_libvirt_nodedev));
+res_dev->device = dev;
+res_dev->conn = conn;
+
+DPRINTF("%s: returning %p\n", PHPFUNC, res_dev->device);
+resource_change_counter(INT_RESOURCE_NODEDEV, conn->conn, res_dev->device,
1 TSRMLS_CC);
+
+VIRT_REGISTER_RESOURCE(res_dev, le_libvirt_nodedev);
+}
+
+/*
+ * Function name: libvirt_nodedev_capabilities
+ * Since version: 0.4.1(-1)
+ * Description: Function is used to list node devices by capabilities
+ * Arguments: @res [resource]: libvirt nodedev resource
+ * Returns: nodedev capabilities array
+ */
+PHP_FUNCTION(libvirt_nodedev_capabilities)
+{
+php_libvirt_nodedev *nodedev = NULL;
+zval *znodedev;
+int count = -1;
+int expectedcount = -1;
+char **names;
+int i;
+
+GET_NODEDEV_FROM_ARGS("r", &znodedev);
+
+if ((expectedcount = virNodeDeviceNumOfCaps(nodedev->device)) < 0)
+RETURN_FALSE;
+names = (char **)emalloc(expectedcount*sizeof(char *));
+count = virNodeDeviceListCaps(nodedev->device, names, expectedcount);
+if ((count != expectedcount) || (count < 0))
+RETURN_FALSE;
+
+array_init(return_value);
+for (i = 0; i < count; i++) {
+VIRT_ADD_NEXT_INDEX_STRING(return_value, names[i]);
+free(names[i]);
+}
+
+efree(names);
+}
+
+/*
+ * Function name: libvirt_nodedev_get_xml_desc
+ * Since version: 0.4.1(-1), changed 0.4.2
+ * Description: Function is used to get the node device's XML description
+ * Arguments: @res [resource]: libvirt nodedev resource
+ * @xpath [string]: optional xPath expression string to get
just this e
[libvirt] [libvirt-php PATCH v2 11/11] Fix is_local_connection implementation.
As it was failing when local host is using FQDN for hostnames. The
logic to do so follows libvirt's implementation for the same thing.
This fixes an issue where unit tests would falsely fail on workstations
that have FQDN hostnames.
---
src/libvirt-php.c | 38 ++
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index b20d839..ec73034 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -1145,13 +1145,43 @@ int is_local_connection(virConnectPtr conn)
{
#ifndef EXTWIN
int ret;
-char *hostname;
+char *lv_hostname = NULL, *result = NULL;
char name[1024];
+struct addrinfo hints, *info = NULL;
-hostname = virConnectGetHostname(conn);
+name[1023] = '\0';
gethostname(name, 1024);
-ret = strcmp(name, hostname) == 0;
-free(hostname);
+
+if (strcmp(name, "localhost") == 0)
+return 1;
+
+lv_hostname = virConnectGetHostname(conn);
+
+/* gethostname gave us FQDN, compare */
+if (strchr(name, '.') && strcmp(name, lv_hostname) == 0)
+return 1;
+
+/* need to get FQDN of the local name */
+memset(&hints, 0, sizeof(hints));
+hints.ai_flags = AI_CANONNAME|AI_CANONIDN;
+hints.ai_family = AF_UNSPEC;
+
+/* could not get FQDN or got localhost, use whatever gethostname gave us */
+if (getaddrinfo(name, NULL, &hints, &info) != 0 ||
+info->ai_canonname == NULL ||
+strcmp(info->ai_canonname, "localhost") == 0)
+result = strdup(name);
+else
+result = strdup(info->ai_canonname);
+
+ret = strcmp(result, lv_hostname) == 0;
+
+freeaddrinfo(info);
+if (lv_hostname)
+free(lv_hostname);
+if (result)
+free(result);
+
return ret;
#else
// Libvirt daemon doesn't work on Windows systems so always return 0
(FALSE)
--
2.13.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-php PATCH v2 02/11] Split up the bindings for libvirt connection API
* add libvirt-connection.h and libvirt-connection.c
* move all libvirt_connect_* function declarations and definitions to
above files
* other minor adjusments to libvirt-php.h and util.h to keep the code
compilable while the code is being moved around.
---
src/Makefile.am | 3 +-
src/libvirt-connection.c | 885 +
src/libvirt-connection.h | 83 +
src/libvirt-php.c| 919 +--
src/libvirt-php.h| 95 ++---
src/util.h | 7 -
6 files changed, 1025 insertions(+), 967 deletions(-)
create mode 100644 src/libvirt-connection.c
create mode 100644 src/libvirt-connection.h
diff --git a/src/Makefile.am b/src/Makefile.am
index bbee667..0819dc6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -21,7 +21,8 @@ libvirt_php_la_SOURCES = \
util.c util.h \
vncfunc.c vncfunc.h \
sockets.c sockets.h \
- libvirt-php.c libvirt-php.h
+ libvirt-php.c libvirt-php.h \
+ libvirt-connection.c libvirt-connection.h
libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
-DCOMPILE_DL_LIBVIRT=1
diff --git a/src/libvirt-connection.c b/src/libvirt-connection.c
new file mode 100644
index 000..bcebd44
--- /dev/null
+++ b/src/libvirt-connection.c
@@ -0,0 +1,885 @@
+/*
+ * libvirt-connection.c: The PHP bindings to libvirt connection API
+ *
+ * See COPYING for the license of this software
+ */
+
+#include
+
+#include "libvirt-connection.h"
+
+DEBUG_INIT("connection");
+
+/*
+ * Private function name: free_resources_on_connection
+ * Since version: 0.4.2
+ * Description: Function is used to free all the resources
assigned to the connection identified by conn
+ * Arguments: @conn [virConnectPtr]: libvirt connection pointer
+ * Returns: None
+ */
+static void
+free_resources_on_connection(virConnectPtr conn TSRMLS_DC)
+{
+int binding_resources_count = 0;
+resource_info *binding_resources;
+int i;
+
+binding_resources_count = LIBVIRT_G(binding_resources_count);
+binding_resources = LIBVIRT_G(binding_resources);
+
+for (i = 0; i < binding_resources_count; i++) {
+if ((binding_resources[i].overwrite == 0) &&
(binding_resources[i].conn == conn))
+free_resource(binding_resources[i].type, binding_resources[i].mem
TSRMLS_CC);
+}
+}
+
+/* Destructor for connection resource */
+void
+php_libvirt_connection_dtor(virt_resource *rsrc TSRMLS_DC)
+{
+php_libvirt_connection *conn = (php_libvirt_connection *) rsrc->ptr;
+int rv = 0;
+
+if (conn != NULL) {
+if (conn->conn != NULL) {
+free_resources_on_connection(conn->conn TSRMLS_CC);
+
+rv = virConnectClose(conn->conn);
+if (rv == -1) {
+DPRINTF("%s: virConnectClose(%p) returned %d (%s)\n",
__FUNCTION__, conn->conn, rv, LIBVIRT_G(last_error));
+php_error_docref(NULL TSRMLS_CC, E_WARNING, "virConnectClose
failed with %i on destructor: %s", rv, LIBVIRT_G(last_error));
+} else {
+DPRINTF("%s: virConnectClose(%p) completed successfully\n",
__FUNCTION__, conn->conn);
+resource_change_counter(INT_RESOURCE_CONNECTION, NULL,
conn->conn, 0 TSRMLS_CC);
+}
+conn->conn = NULL;
+}
+efree(conn);
+}
+}
+
+/* Authentication callback function.
+ *
+ * Should receive list of credentials via cbdata and pass the requested one to
+ * libvirt
+ */
+static int libvirt_virConnectAuthCallback(virConnectCredentialPtr cred,
+ unsigned int ncred, void *cbdata)
+{
+TSRMLS_FETCH();
+
+unsigned int i, j;
+php_libvirt_cred_value *creds = (php_libvirt_cred_value *) cbdata;
+for (i = 0; i < (unsigned int)ncred; i++) {
+DPRINTF("%s: cred %d, type %d, prompt %s challenge %s\n ",
__FUNCTION__, i, cred[i].type, cred[i].prompt, cred[i].challenge);
+if (creds != NULL)
+for (j = 0; j < (unsigned int)creds[0].count; j++) {
+if (creds[j].type == cred[i].type) {
+cred[i].resultlen = creds[j].resultlen;
+cred[i].result = (char *)malloc(creds[j].resultlen + 1);
+memset(cred[i].result, 0, creds[j].resultlen + 1);
+strncpy(cred[i].result, creds[j].result,
creds[j].resultlen);
+}
+}
+DPRINTF("%s: result %s (%d)\n", __FUNCTION__, cred[i].result,
cred[i].resultlen);
+}
+
+return 0;
+}
+
+static int libvirt_virConnectCredType[] = {
+VIR_CRED_AUTHNAME,
+VIR_CRED_ECHOPROMPT,
+VIR_CRED_REALM,
+VIR_CRED_PASSPHRASE,
+VIR_CRED_NOECHOPROMPT,
+//VIR_CRED_EXTERNAL,
+};
+
+/*
+ * Function name: libvirt_connect
+ * Since version: 0.4.1(-1)
+ * Description: libvirt_connect() is used to connect to the specified
libvirt daemon using the specified
[libvirt] [libvirt-php PATCH v2 00/11] Refactor into smaller components
As per [1], this patch series splits up the large libvirt-php.c into components that (attempts) to resemble the structure of the libvirt project. Each patch successive patch was compile-tested while the whole series was verified with "make check" and a simple custom written PHP script. Changes from v1 [2]: * rebase on master * include PHP headers in util.h instead of libvirt-php.h this makes header inter-dependencies easier to manage/understand * also test each patch on PHP 5 [1] https://www.redhat.com/archives/libvir-list/2017-June/msg00991.html [2] https://www.redhat.com/archives/libvir-list/2017-August/msg00046.html Dawid Zamirski (11): Move PHP version compat macros to utils.h Split up the bindings for libvirt connection API Split up the bindings for libvirt node API Split up the bindings for libvirt stream API Split up the bindings for libvirt domain API Split up the bindings for libvirt network API Split up the bindings for libvirt storage API Split up the bindings for libvirt snapshot API Split up the bindings for libvirt nodedev API Split up the bindings for libvirt NWFilter API Fix is_local_connection implementation. src/Makefile.am | 11 +- src/libvirt-connection.c | 886 + src/libvirt-connection.h | 81 + src/libvirt-domain.c | 3344 + src/libvirt-domain.h | 208 ++ src/libvirt-network.c| 587 +++ src/libvirt-network.h| 73 + src/libvirt-node.c | 305 ++ src/libvirt-node.h | 23 + src/libvirt-nodedev.c| 340 ++ src/libvirt-nodedev.h| 54 + src/libvirt-nwfilter.c | 415 +++ src/libvirt-nwfilter.h | 66 + src/libvirt-php.c| 9277 -- src/libvirt-php.h| 496 +-- src/libvirt-snapshot.c | 244 ++ src/libvirt-snapshot.h | 58 + src/libvirt-storage.c| 1130 ++ src/libvirt-storage.h| 137 + src/libvirt-stream.c | 230 ++ src/libvirt-stream.h | 39 + src/util.h | 200 +- 22 files changed, 9282 insertions(+), 8922 deletions(-) create mode 100644 src/libvirt-connection.c create mode 100644 src/libvirt-connection.h create mode 100644 src/libvirt-domain.c create mode 100644 src/libvirt-domain.h create mode 100644 src/libvirt-network.c create mode 100644 src/libvirt-network.h create mode 100644 src/libvirt-node.c create mode 100644 src/libvirt-node.h create mode 100644 src/libvirt-nodedev.c create mode 100644 src/libvirt-nodedev.h create mode 100644 src/libvirt-nwfilter.c create mode 100644 src/libvirt-nwfilter.h create mode 100644 src/libvirt-snapshot.c create mode 100644 src/libvirt-snapshot.h create mode 100644 src/libvirt-storage.c create mode 100644 src/libvirt-storage.h create mode 100644 src/libvirt-stream.c create mode 100644 src/libvirt-stream.h -- 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RESEND] Interface: return appropriate status for bridge device
On 08/02/2017 05:09 PM, John Ferlan wrote:
> [pardon the top-post comment]
> Doing some (personal) mail box cleaning and found this "older" patch...
> Sorry that this has sat unattended (and probably now forgotten or given
> up on) for so long. Suppose it's an assumption that someone like Laine
> who understand bonds/bridges better than others would pick it up...
>
> Although, it's not my space, I'll provide some feedback in general and
> if you want to rework, send again - then hopefully someone more in the
> know about "expectations" can look at it...
>
> On 02/13/2017 05:06 AM, Lin Ma wrote:
>> In function udevInterfaceIsActive, The current design relies on the sys
>> attribute 'operstate' to determine whether the interface is active.
>>
>> For the device node of virtual network(say virbr0), There is already a
>> dummy tap device to maintain a fixed mac on it, So it's available and
>> its status should be considered as active. But if no anyelse tap device
> anyelse needs to be adjusted.
>
>> connects to it, The value of operstate of virbr0 in sysfs is 'down', So
>> udevInterfaceIsActive returns 0, It causes 'virsh iface-list --all' to
>> report the status of virbr0 as 'inactive'.
> Personally, I'm not sure if this is a feature or a problem, hopefully
> Laine can elaborate!
Actually it's a problem that the udev-based interface driver lists
transient interfaces like virbr0 *at all*. The netcf backend for the
interface driver only shows interfaces that are in the host's persistent
system network config, it doesn't list anything created by libvirt's
virtual network drivers - the two spaces are supposed to be separate
from each other. This was one of the problems I had with the udev
backend for the interface driver back when it was written - it isn't
providing the same list of devices that the netcf backend does, and that
means that consumers like virt-manager will show different behavior on
different distros (e.g. it will either erroneously show virbr0 as a
bridge that you might want to connect a guest to directly, or show some
other strange transient interface as a potential connection for macvtap).
The basic problem, of course, is that netcf looks to the system network
config to get the canonical list of network interfaces' and their
persistent configuration, while the udev backend looks at the current
status of network interfaces via udev and sysfs, paying no attention to
persistent config.
In the end, if you really care about the online status of virbr0 as
provided by iface-dumpxml, then you're using the virInterface APIs in a
way that will behave differently on different distros, and I don't know
how good of an idea that is.
If, on the other hand, you're seeing a problem with bridge devices that
are in the host system's persistent network config (e.g. a bridge that's
attached to a physical ethernet), then I guess I can see the utility of
getting this right (although in that case, it would be following the
operstate of the physical ethernet anyway, wouldn't it?)
Beyond that, because my position has always been that distros should
either use netcf, or at least use a backend with the same semantics as
netcf rather than something that's just providing a "similar
approximation" of netcf semantics, I don't really have much to say about
the internal details. This is one of those cases where "if it works, and
doesn't make the already-objectionable code any more objectionable, then
sure, whatever".
Sorry if I'm not more enthusiastic about this code (not your patches,
but the code that you're applying the patches to). I've just never been
a fan.
>> This patch fixes the issue by counting slave devices for bridge device.
>>
>> Signed-off-by: Lin Ma
>> ---
>> src/interface/interface_backend_udev.c | 22 ++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/src/interface/interface_backend_udev.c
>> b/src/interface/interface_backend_udev.c
>> index 5d0fc64..9ac4674 100644
>> --- a/src/interface/interface_backend_udev.c
>> +++ b/src/interface/interface_backend_udev.c
>> @@ -1127,6 +1127,11 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
>> struct udev_device *dev;
>> virInterfaceDefPtr def = NULL;
>> int status = -1;
>> +struct dirent **member_list = NULL;
>> +const char *devtype;
>> +int member_count = 0;
>> +char *member_path;
>> +size_t i;
>>
>> dev = udev_device_new_from_subsystem_sysname(udev, "net",
>> ifinfo->name);
>> @@ -1146,6 +1151,23 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
>> /* Check if it's active or not */
>> status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
>>
> The following hunk should be in a "helper" - that is create another
> function...
>
> if (!status) {
> devtype = udev_device_get_devtype(dev);
> if (STREQ_NULLABLE(devtype, "bridge"))
> status = newProperlyNamedHelper(arg1, arg2);
> }
>
> Alt
Re: [libvirt] [PATCH] qemu: Honour
Hello, Am 03.08.2017 um 09:36 schrieb Michal Privoznik: > https://bugzilla.redhat.com/show_bug.cgi?id=1476866 > > For some reason, we completely ignore setting for > domains. The implementation is simply not there. It never was. > However, things are slightly more complicated. QEMU sends us two > RESET events on domain reboot. Fortunately, the event contains > this 'guest' field telling us who initiated the reboot. And since > we don't want to destroy the domain if the reset is initiated by > a user, we have to ignore those events. Whatever, just look at > the code. White you are at "QEMU reset": From Xen I remember that on reboot a new qemu-dm (Device Model) is created - if I remember correctly - for both PV and HV. For QEMU the old qemu process is reused and the reset is done by SeaBios inside the VM. If would be cool if there was an option to kill the old qemu process and start a new qemu process (with an updated configuration) on reboot. I sometimes have the situation where the libvirt part is done by one group of admins, while the guest OS and everything within in VM is done by some other group of persons. Currently they always have to coordinate a time, where the internal group does initiate the guest OS shutdown and the libvirt admins then updates the configuration and starts the VM again. It would be nice if I could update the config "just now" and then tell the OS group "just do the reboot when your schedule permits it - you will then get your updates configuration automatically." Or is this already there and I missed it? Philipp -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php PATCH 04/13] Split up the bindings for libvirt connection API
On Thu, 2017-08-03 at 17:02 +0200, Michal Privoznik wrote: > On 08/03/2017 04:58 PM, Dawid Zamirski wrote: > > > > On Thu, 2017-08-03 at 14:27 +0200, Michal Privoznik wrote: > > > On 08/01/2017 11:46 PM, Dawid Zamirski wrote: > > > > * add libvirt-connection.h and libvirt-connection.c > > > > * move all libvirt_connect_* function declarations and > > > > definitions > > > > to > > > > above files > > > > * other minor adjusments to libvirt-php.h and util.h to keep > > > > the > > > > code > > > > compilable while the code is being moved around. > > > > --- > > > > src/Makefile.am | 3 +- > > > > src/libvirt-connection.c | 885 > > > > + > > > > src/libvirt-connection.h | 83 + > > > > src/libvirt-php.c| 919 + > > > > > > > > -- > > > > src/libvirt-php.h| 104 +++--- > > > > src/util.h | 7 - > > > > 6 files changed, 1024 insertions(+), 977 deletions(-) > > > > create mode 100644 src/libvirt-connection.c > > > > create mode 100644 src/libvirt-connection.h > > > > > > Unfortunately, this breaks the build for me. I'm compiling with > > > php5.6. > > > Moreover, as I try to fix the build I wonder if we can put all > > > the > > > php > > > differentiating code into util.h - it'd need to include php.h > > > then > > > (for > > > all those ZEND_* macros and zend_* types). > > > > > > I'm pushing patches 02 and 03 meanwhile. > > > > > > Michal > > > > Sure, I'll do that - it will get rid of the circular dependency > > between > > libvirt-php.h and util.h which I didn't like too much anyway. I'll > > also > > re-test everything on PHP 5 before sending v2. > > Oh cool. I didn't expect you to care that much :-) Feel free to > arrange > headers whatever you like and whatever works. It's more important to > split the libvirt-php.c than anything. Thanks! > > Michal Yep ;-) ... and that's precisely why PHP 5.x does not compile util.h typdefs long to zend_long for PHP < 7, libvirt-php.h uses zend_long but does not include util.h - so yeah it wasn't quite circular dependency but still a mess. Dawid -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] tests: add qemu x86 kvm 32-on-64 test
There's some specific logic in qemuBuildCpuCommandLine to support
auto adding -cpu qemu 32 for arch=i686 with an x86_64 qemu binary.
Add a test case for it
Signed-off-by: Cole Robinson
---
v2:
Drop unnecessary qemu-kvm path usage
.../qemuxml2argv-x86-kvm-32-on-64.args | 21 +
.../qemuxml2argv-x86-kvm-32-on-64.xml | 13 +
tests/qemuxml2argvtest.c| 1 +
3 files changed, 35 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
new file mode 100644
index 0..5b644b0a6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
@@ -0,0 +1,21 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name kvm \
+-S \
+-machine pc,accel=kvm \
+-cpu qemu32 \
+-m 4096 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid d091ea82-29e6-2e34-3005-f02617b36e87 \
+-nographic \
+-nodefaults \
+-chardev
socket,id=charmonitor,path=/tmp/lib/domain--1-kvm/monitor.sock,server,\
+nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
new file mode 100644
index 0..37f53bf2a
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
@@ -0,0 +1,13 @@
+
+ kvm
+ d091ea82-29e6-2e34-3005-f02617b36e87
+ 4194304
+
+hvm
+
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index aa83013a2..f74d45ba0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -709,6 +709,7 @@ mymain(void)
DO_TEST("kvm", QEMU_CAPS_MACHINE_OPT);
DO_TEST("default-kvm-host-arch", QEMU_CAPS_MACHINE_OPT);
DO_TEST("default-qemu-host-arch", QEMU_CAPS_MACHINE_OPT);
+DO_TEST("x86-kvm-32-on-64", QEMU_CAPS_MACHINE_OPT);
DO_TEST("boot-cdrom", NONE);
DO_TEST("boot-network", NONE);
DO_TEST("boot-floppy", NONE);
--
2.13.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] tests: add qemu x86 kvm 32-on-64 test
On 07/21/2017 07:40 AM, Pavel Hrdina wrote:
> On Fri, Jul 14, 2017 at 07:43:04PM -0400, Cole Robinson wrote:
>> There's some specific logic in qemuBuildCpuCommandLine to support
>> auto adding -cpu qemu 32 for arch=i686 with an x86_64 qemu-kvm binary.
>> Add a test case for it
>>
>> Signed-off-by: Cole Robinson
>> ---
>> .../qemuxml2argv-x86-kvm-32-on-64.args | 21
>> +
>> .../qemuxml2argv-x86-kvm-32-on-64.xml | 13 +
>> tests/qemuxml2argvtest.c| 1 +
>> tests/testutilsqemu.c | 18 --
>> 4 files changed, 51 insertions(+), 2 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>> b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>> new file mode 100644
>> index 0..5fdeaf843
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.args
>> @@ -0,0 +1,21 @@
>> +LC_ALL=C \
>> +PATH=/bin \
>> +HOME=/home/test \
>> +USER=test \
>> +LOGNAME=test \
>> +QEMU_AUDIO_DRV=none \
>> +/usr/bin/qemu-kvm \
>> +-name kvm \
>> +-S \
>> +-machine pc,accel=kvm \
>> +-cpu qemu32 \
>> +-m 4096 \
>> +-smp 1,sockets=1,cores=1,threads=1 \
>> +-uuid d091ea82-29e6-2e34-3005-f02617b36e87 \
>> +-nographic \
>> +-nodefaults \
>> +-chardev
>> socket,id=charmonitor,path=/tmp/lib/domain--1-kvm/monitor.sock,server,\
>> +nowait \
>> +-mon chardev=charmonitor,id=monitor,mode=readline \
>> +-no-acpi \
>> +-boot c
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>> b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>> new file mode 100644
>> index 0..2939cec15
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-x86-kvm-32-on-64.xml
>> @@ -0,0 +1,13 @@
>> +
>> + kvm
>> + d091ea82-29e6-2e34-3005-f02617b36e87
>> + 4194304
>> +
>> +hvm
>> +
>> +
>> +/usr/bin/qemu-kvm
>> +
>> +
>> +
>> +
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 302c9c892..ef5a9b0dc 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -690,6 +690,7 @@ mymain(void)
>> DO_TEST("kvm", QEMU_CAPS_MACHINE_OPT);
>> DO_TEST("default-kvm-host-arch", QEMU_CAPS_MACHINE_OPT);
>> DO_TEST("default-qemu-host-arch", QEMU_CAPS_MACHINE_OPT);
>> +DO_TEST("x86-kvm-32-on-64", QEMU_CAPS_MACHINE_OPT);
>> DO_TEST("boot-cdrom", NONE);
>> DO_TEST("boot-network", NONE);
>> DO_TEST("boot-floppy", NONE);
>> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
>> index ee4853841..d1290fdde 100644
>> --- a/tests/testutilsqemu.c
>> +++ b/tests/testutilsqemu.c
>> @@ -111,7 +111,8 @@ typedef enum {
>> TEST_UTILS_QEMU_BIN_ARM,
>> TEST_UTILS_QEMU_BIN_PPC64,
>> TEST_UTILS_QEMU_BIN_PPC,
>> -TEST_UTILS_QEMU_BIN_S390X
>> +TEST_UTILS_QEMU_BIN_S390X,
>> +TEST_UTILS_QEMU_BIN_KVM,
>> } QEMUBinType;
>>
>> static const char *QEMUBinList[] = {
>> @@ -121,7 +122,8 @@ static const char *QEMUBinList[] = {
>> "/usr/bin/qemu-system-arm",
>> "/usr/bin/qemu-system-ppc64",
>> "/usr/bin/qemu-system-ppc",
>> -"/usr/bin/qemu-system-s390x"
>> +"/usr/bin/qemu-system-s390x",
>> +"/usr/bin/qemu-kvm",
>
> I don't like this patch because it adds a binary that is specific to
> downstream releases. The whole point of my previous patches was to
> remove all downstream binaries from our test suite.
>
> This test can be easily done with the current x86_64 binary
> TEST_UTILS_QEMU_BIN_X86_64.
>
I thought the qemu-kvm patch was required to trigger the logic, but indeed
it's not necessary. I'll send a v2
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/8] Some virObjectRW* adjustments
On 08/03/2017 11:34 AM, Michal Privoznik wrote:
> On 08/01/2017 02:05 AM, John Ferlan wrote:
>> v1: https://www.redhat.com/archives/libvir-list/2017-July/msg01313.html
>>
>> Differences from v1:
>>
>> * Use the names virObjectRWLockRead, virObjectRWLockWrite and
>>virObjectRWUnlock
>>
>> * Instead of an 'int' return, make the virObjectRWLock{Read|Write} be
>>void returns just like virObject{Lock|Unlock}
>>
>> * Separate out the magic number check for the virObjectIsClass consumers
>>into its own patch and describe the reasons for usage.
>>
>> * Apply the same magic number check to virObject{Ref|Unref} separately.
>>
>> BTW: This looks and works eally nice with what I have for common objects...
>>
>> John Ferlan (8):
>> util: Rename virObjectLockRead to virObjectRWLockRead
>> util: Introduce and use virObjectRWLockWrite
>> util: Only have virObjectLock handle virObjectLockable
>> util: Introduce virObjectGetRWLockableObj
>> util: Introduce and use virObjectRWUnlock
>> util: Create common error path for invalid object
>> util: Add magic number check for object validity
>> util: Add object checking for virObject{Ref|Unref}
>>
>> src/conf/virdomainobjlist.c | 62
>> src/libvirt_private.syms| 4 +-
>> src/util/virobject.c| 169
>> +---
>> src/util/virobject.h| 10 ++-
>> 4 files changed, 169 insertions(+), 76 deletions(-)
>>
>
> Okay, I've ran some local tests and indeed no tool showed any
> misbehaviour when my test binary was mutex-locking an RW lock or
> rwlocking a mutex. While I still believe that us, reviewers have to be
> careful around locks anyway, rename to virObjectRWLock* can help us
> remember if we forget.
>
> ACK series.
>
> Michal
>
Thanks - I'll hold off on pushing though... I'm fine with using
VIR_ERROR instead of VIR_WARN too...
In any case, I'm away next week and wouldn't want to leave that kind of
present for anyone to revert! Besides, I'm assuming there may be other
opinions and there isn't a "rush" to get this in as nothing is broken yet...
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: command: explicitly error for non-x86 default CPU
On 07/21/2017 10:25 AM, Ján Tomko wrote: > On Fri, Jul 14, 2017 at 07:43:06PM -0400, Cole Robinson wrote: >> The code only currently handles writing an x86 default -cpu >> argument, and doesn't know anything about other architectures. >> Let's make this explicit rather than leaving ex. qemu ppc64 to >> throw an error about -cpu qemu64 >> >> Signed-off-by: Cole Robinson >> --- >> src/qemu/qemu_command.c | 25 ++--- >> 1 file changed, 18 insertions(+), 7 deletions(-) >> > > ACK > > Jan Thanks, I pushed patch 2 + 3 now which you reviewed, patch 1 was independent and has some comments. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/8] util: Only have virObjectLock handle virObjectLockable
On 08/01/2017 02:05 AM, John Ferlan wrote:
> Now that virObjectRWLockWrite exists to handle the virObjectRWLockable
> objects, let's restore virObjectLock to only handle virObjectLockable
> class locks. There still exists the possibility that the input @anyobj
> isn't a valid object and the resource isn't truly locked, but that
> also exists before commit id '77f4593b'.
>
> This also restores some logic that commit id '77f4593b' removed
> with respect to a common code path that commit id '10c2bb2b' had
> introduced as virObjectGetLockableObj. This code path merely does
> the same checks as the original virObjectLock commit 'b545f65d',
> but in callable/reusable helper to ensure the @obj at least has
> some validity before using.
>
> Signed-off-by: John Ferlan
> ---
> src/util/virobject.c | 37 +++--
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index f49af62..4903393 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -367,13 +367,28 @@ virObjectRef(void *anyobj)
> }
>
>
> +static virObjectLockablePtr
> +virObjectGetLockableObj(void *anyobj)
> +{
> +virObjectPtr obj;
> +
> +if (virObjectIsClass(anyobj, virObjectLockableClass))
> +return anyobj;
> +
> +obj = anyobj;
> +VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
> + anyobj, obj ? obj->klass->name : "(unknown)");
If we're really worried about this (and don't want to abort()), we might
consider raising this to VIR_ERROR. I'm a programmer, I don't care about
warnings O:-)
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/8] Some virObjectRW* adjustments
On 08/01/2017 02:05 AM, John Ferlan wrote:
> v1: https://www.redhat.com/archives/libvir-list/2017-July/msg01313.html
>
> Differences from v1:
>
> * Use the names virObjectRWLockRead, virObjectRWLockWrite and
>virObjectRWUnlock
>
> * Instead of an 'int' return, make the virObjectRWLock{Read|Write} be
>void returns just like virObject{Lock|Unlock}
>
> * Separate out the magic number check for the virObjectIsClass consumers
>into its own patch and describe the reasons for usage.
>
> * Apply the same magic number check to virObject{Ref|Unref} separately.
>
> BTW: This looks and works eally nice with what I have for common objects...
>
> John Ferlan (8):
> util: Rename virObjectLockRead to virObjectRWLockRead
> util: Introduce and use virObjectRWLockWrite
> util: Only have virObjectLock handle virObjectLockable
> util: Introduce virObjectGetRWLockableObj
> util: Introduce and use virObjectRWUnlock
> util: Create common error path for invalid object
> util: Add magic number check for object validity
> util: Add object checking for virObject{Ref|Unref}
>
> src/conf/virdomainobjlist.c | 62
> src/libvirt_private.syms| 4 +-
> src/util/virobject.c| 169
> +---
> src/util/virobject.h| 10 ++-
> 4 files changed, 169 insertions(+), 76 deletions(-)
>
Okay, I've ran some local tests and indeed no tool showed any
misbehaviour when my test binary was mutex-locking an RW lock or
rwlocking a mutex. While I still believe that us, reviewers have to be
careful around locks anyway, rename to virObjectRWLock* can help us
remember if we forget.
ACK series.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] qemu: Restrict usage of hpet and kvm.pit timers by unsupported architectures
On 07/25/2017 10:36 AM, Kothapally Madhu Pavan wrote:
> hpet and kvm.pit clock timers are specific to x86 architecture and
> are not suppose to be used in other architectures. This patch restricts
> the usage of hpet and kvm.pit timers in unsupported architectures.
>
> Signed-off-by: Kothapally Madhu Pavan
> ---
> src/qemu/qemu_domain.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2c8c9a7..e5e4208 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3083,12 +3083,28 @@ qemuDomainDefValidate(const virDomainDef *def,
> virQEMUCapsPtr qemuCaps = NULL;
> unsigned int topologycpus;
> int ret = -1;
> +size_t i;
>
> if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
> driver->qemuCapsCache,
> def->emulator)))
> goto cleanup;
>
> +/* Restrict usage of unsupported clock sources */
> +for (i = 0; i < def->clock.ntimers; i++) {
> +virDomainTimerDefPtr timer = def->clock.timers[i];
> +if ((!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) &&
> + (timer->name == VIR_DOMAIN_TIMER_NAME_HPET)) ||
> +(!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) &&
> + (timer->name == VIR_DOMAIN_TIMER_NAME_PIT))) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported clock timer '%s' for %s
> architecture"),
> +
> virDomainTimerNameTypeToString(def->clock.timers[i]->name),
> + virArchToString(def->os.arch));
> +goto cleanup;
> +}
> +}
> +
> if (def->mem.min_guarantee) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("Parameter 'min_guarantee' not supported by
> QEMU."));
>
The HPET bit isn't strictly correct. See the HPET handling in qemu_command.c,
the only case that's explicitly rejected is when hpet present=yes is
specified, but qemu doesn't have NO_HPET. For example requesting hpet
present=no on PPC64 is arguably a valid request, since there won't ever be
hpet available.
So I think in this case it's better to just drop the HPET checking, OR move
the qemu_command.c error message here. But the benefit of this is very small
(validating hpet present=yes at XML define time vs startup time)
However patch #2 will also reject that HPET case. I think patch #2 should be
dropped entirely. Figure out what timer XML settings produce ambiguous qemu
results (the kvm-pit bit you mentioned previously that qemu won't error about)
and only reject those, and do it with QEMU_CAPS settings.
If qemu/libvirt already throws an error, like it seems to do for hypervclock,
tsc, kvmclock, hpet, platform, then adding an extra validation only adds the
benefit of an improved error message and catching the issue at XML parse time,
but the downside is it runs the risk of being overly restrictive, and it adds
more code to maintain. So IMO for those cases that are going to error anyway
it's typically better to just leave it alone unless the qemu error message is
really ambiguous
For the kvmclock/hypervclock/tsc issue, I posted patches to try and improve
our handling a bit and give a more explicit error, still need to address
reviews/push them though
https://www.redhat.com/archives/libvir-list/2017-July/msg00537.html
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix smartcard formatting regression
On 08/03/2017 08:55 AM, Ján Tomko wrote: > *** BLURB HERE *** > > Ján Tomko (2): > tests: add smartcard test cases to qemuxml2xmltest > conf: fix formatting of smartcard devices > > src/conf/domain_conf.c | 43 > ++ > .../qemuxml2xmlout-smartcard-controller.xml| 31 > .../qemuxml2xmlout-smartcard-host-certificates.xml | 34 + > .../qemuxml2xmlout-smartcard-host.xml | 31 > ...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31 > .../qemuxml2xmlout-smartcard-passthrough-tcp.xml | 33 + > tests/qemuxml2xmltest.c| 6 +++ > 7 files changed, 194 insertions(+), 15 deletions(-) > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml > create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml > I'm fine with this solution - instead of what I posted: https://www.redhat.com/archives/libvir-list/2017-August/msg00101.html The only differences between the two are: 1. Where the virBufferAddLit(buf, ">\n"); is placed for all callers 2. How the smartcard passthrough is processed I posted mainly as a means to have "something" available - I can drop my series... Reviewed-by: John Ferlan Although I assume you're going to merge them together so as to not break git bisect, right? John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php PATCH 04/13] Split up the bindings for libvirt connection API
On 08/03/2017 04:58 PM, Dawid Zamirski wrote: > > On Thu, 2017-08-03 at 14:27 +0200, Michal Privoznik wrote: >> On 08/01/2017 11:46 PM, Dawid Zamirski wrote: >>> * add libvirt-connection.h and libvirt-connection.c >>> * move all libvirt_connect_* function declarations and definitions >>> to >>> above files >>> * other minor adjusments to libvirt-php.h and util.h to keep the >>> code >>> compilable while the code is being moved around. >>> --- >>> src/Makefile.am | 3 +- >>> src/libvirt-connection.c | 885 >>> + >>> src/libvirt-connection.h | 83 + >>> src/libvirt-php.c| 919 + >>> -- >>> src/libvirt-php.h| 104 +++--- >>> src/util.h | 7 - >>> 6 files changed, 1024 insertions(+), 977 deletions(-) >>> create mode 100644 src/libvirt-connection.c >>> create mode 100644 src/libvirt-connection.h >> >> Unfortunately, this breaks the build for me. I'm compiling with >> php5.6. >> Moreover, as I try to fix the build I wonder if we can put all the >> php >> differentiating code into util.h - it'd need to include php.h then >> (for >> all those ZEND_* macros and zend_* types). >> >> I'm pushing patches 02 and 03 meanwhile. >> >> Michal > > Sure, I'll do that - it will get rid of the circular dependency between > libvirt-php.h and util.h which I didn't like too much anyway. I'll also > re-test everything on PHP 5 before sending v2. Oh cool. I didn't expect you to care that much :-) Feel free to arrange headers whatever you like and whatever works. It's more important to split the libvirt-php.c than anything. Thanks! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php PATCH 04/13] Split up the bindings for libvirt connection API
On Thu, 2017-08-03 at 14:27 +0200, Michal Privoznik wrote: > On 08/01/2017 11:46 PM, Dawid Zamirski wrote: > > * add libvirt-connection.h and libvirt-connection.c > > * move all libvirt_connect_* function declarations and definitions > > to > > above files > > * other minor adjusments to libvirt-php.h and util.h to keep the > > code > > compilable while the code is being moved around. > > --- > > src/Makefile.am | 3 +- > > src/libvirt-connection.c | 885 > > + > > src/libvirt-connection.h | 83 + > > src/libvirt-php.c| 919 + > > -- > > src/libvirt-php.h| 104 +++--- > > src/util.h | 7 - > > 6 files changed, 1024 insertions(+), 977 deletions(-) > > create mode 100644 src/libvirt-connection.c > > create mode 100644 src/libvirt-connection.h > > Unfortunately, this breaks the build for me. I'm compiling with > php5.6. > Moreover, as I try to fix the build I wonder if we can put all the > php > differentiating code into util.h - it'd need to include php.h then > (for > all those ZEND_* macros and zend_* types). > > I'm pushing patches 02 and 03 meanwhile. > > Michal Sure, I'll do that - it will get rid of the circular dependency between libvirt-php.h and util.h which I didn't like too much anyway. I'll also re-test everything on PHP 5 before sending v2. Dawid -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] libxl: Add a test suite for libxl_domain_config generator
On Tue, Aug 01, 2017 at 03:17:51PM +0200, Marek Marczykowski-Górecki wrote: > From: Jim Fehlig > > The libxl library allows a libxl_domain_config object to be serialized > from/to a JSON string. Use this to allow testing of the XML to > libxl_domain_config conversion process. Test XML is converted to > libxl_domain_config, which is then serialized to json. A json template > corresponding to the test XML is converted to a libxl_domain_config > object using libxl_domain_config_from_json(), and then serialized > back to json using libxl_domain_config_to_json(). The two json > docs are then compared. > > Using libxl to convert the json template to a libxl_domain_config > object and then back to json provides a simple way to account for > any changes or additions to the json representation across Xen > releases. > > Signed-off-by: Jim Fehlig > [update to v3.5.0-rc1, improve error reporting, use /bin/true emulator] > Signed-off-by: Marek Marczykowski-Górecki I've been looking at this series for the past days, and taking into account the comments that Jim mentioned yesterday are ammended, and this looks good to me: Reviewed-by: Joao Martins It adds a really nice piece of testing infra for libxl_domain_configs. Maybe in the future more tests could be added in (in addition to the CPUID one that Marek has planned). Joao > --- > m4/virt-driver-libxl.m4| 6 +- > tests/Makefile.am | 18 +- > tests/libxlxml2domconfigdata/basic-hvm.json| 89 - > tests/libxlxml2domconfigdata/basic-hvm.xml | 36 +++- > tests/libxlxml2domconfigdata/basic-pv.json | 65 ++- > tests/libxlxml2domconfigdata/basic-pv.xml | 28 ++- > tests/libxlxml2domconfigdata/moredevs-hvm.json | 111 ++- > tests/libxlxml2domconfigdata/moredevs-hvm.xml | 63 ++- > tests/libxlxml2domconfigtest.c | 205 ++- > tests/virmocklibxl.c | 87 - > 10 files changed, 705 insertions(+), 3 deletions(-) > create mode 100644 tests/libxlxml2domconfigdata/basic-hvm.json > create mode 100644 tests/libxlxml2domconfigdata/basic-hvm.xml > create mode 100644 tests/libxlxml2domconfigdata/basic-pv.json > create mode 100644 tests/libxlxml2domconfigdata/basic-pv.xml > create mode 100644 tests/libxlxml2domconfigdata/moredevs-hvm.json > create mode 100644 tests/libxlxml2domconfigdata/moredevs-hvm.xml > create mode 100644 tests/libxlxml2domconfigtest.c > create mode 100644 tests/virmocklibxl.c > > diff --git a/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4 > index 96a9d47..3d635f0 100644 > --- a/m4/virt-driver-libxl.m4 > +++ b/m4/virt-driver-libxl.m4 > @@ -35,7 +35,7 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ >if test "x$with_libxl" = "xyes" ; then > LIBXL_FIRMWARE_DIR=$($PKG_CONFIG --variable xenfirmwaredir xenlight) > LIBXL_EXECBIN_DIR=$($PKG_CONFIG --variable libexec_bin xenlight) > - fi > + fi > >dnl pkgconfig file not found, fallback to lib probe >if test "x$with_libxl" = "xno" ; then > @@ -80,6 +80,10 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [ >LIBXL_LIBS="$LIBXL_LIBS -lxenctrl" > ]) > > +dnl Check if libxl_domain_config_from_json is available for domXML to > +dnl libxl_domain_config tests > +LIBS="$LIBS -lxenlight -lxenctrl" > +AC_CHECK_FUNCS([libxl_domain_config_from_json]) > CFLAGS="$old_CFLAGS" > LIBS="$old_LIBS" >fi > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 3e3d580..49b9d40 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -99,6 +99,7 @@ EXTRA_DIST =\ > genericxml2xmlindata \ > genericxml2xmloutdata \ > interfaceschemadata \ > + libxlxml2domconfigdata \ > lxcconf2xmldata \ > lxcxml2xmldata \ > lxcxml2xmloutdata \ > @@ -271,7 +272,8 @@ test_programs += xml2sexprtest sexpr2xmltest \ > endif WITH_XEN > > if WITH_LIBXL > -test_programs += xlconfigtest > +test_programs += xlconfigtest libxlxml2domconfigtest > +test_libraries += virmocklibxl.la > endif WITH_LIBXL > > if WITH_QEMU > @@ -528,8 +530,20 @@ xlconfigtest_SOURCES = \ > xlconfigtest.c testutilsxen.c testutilsxen.h \ > testutils.c testutils.h > xlconfigtest_LDADD =$(libxl_LDADDS) > + > +libxlxml2domconfigtest_SOURCES = \ > + libxlxml2domconfigtest.c testutilsxen.c testutilsxen.h \ > + testutils.c testutils.h > +libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS) > + > +virmocklibxl_la_SOURCES = \ > + virmocklibxl.c > +virmocklibxl_la_CFLAGS = $(AM_CFLAGS) > +virmocklibxl_la_LDFLAGS = -module -avoid-version \ > +-rpath /evil/libtool/hack/to/force/shared/lib/creation > + > else ! WITH_LIBXL > -EXTRA_DIST += xlconfigtest.c > +EXTRA_DIST += xlconfigtest.c libxlxml2domconfigtest.c > endif ! WITH_LIBXL > > QEMUMONITORTESTUTILS_SOURCES = \ > diff --git a/tests/libxlxml2domconfigdata/basic-hvm.json > b/tests/libxlxml2domconfigdata/
Re: [libvirt] [PATCH v2 1/8] util: Rename virObjectLockRead to virObjectRWLockRead
On 08/01/2017 05:36 PM, John Ferlan wrote:
>
>
> On 08/01/2017 09:24 AM, Michal Privoznik wrote:
>> On 08/01/2017 02:05 AM, John Ferlan wrote:
>>> Since the class it represents is based on virObjectRWLockableClass
>>> and in order to make sure we diffentiate lest anyone somehow
>>
>> ^^ couple of typos
>>
>
> Just differentiate from my dictionary.
>
> 'lest' is someone colloquial - I can alter it though.
Ah, okay. I'm no native speaker. Learned something new :-)
>
>>> believes they could use virObjectLockRead for a virObjectLockableClass,
>>> let's rename the API to use the RW in the name. Besides the RW locks
>>> refer to pthread_rwlock_{init|rdlock|wrlock|unlock|destroy} while the
>>> other locks refer to pthread_mutex_{init|lock|unlock|destroy}.
>>
>> Firstly, this is just an internal implementation. We often rename APIs
>> for us to use. Secondly, this is because pthreads are C API with no
>> 'object' reference. So they have to have two unlock functions for two
>> different objects.
>>
>
> Well function naming guidelines weren't in place when virObjectLock (and
> Unlock) were added, but they are now.
>
>>>
>>> Signed-off-by: John Ferlan
>>> ---
>>> src/conf/virdomainobjlist.c | 18 +-
>>> src/libvirt_private.syms| 2 +-
>>> src/util/virobject.c| 11 ---
>>> src/util/virobject.h| 2 +-
>>> 4 files changed, 19 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
>>> index 1d027a4..9bc6603 100644
>>> --- a/src/conf/virdomainobjlist.c
>>> +++ b/src/conf/virdomainobjlist.c
>>> @@ -118,7 +118,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr
>>> doms,
>>> bool ref)
>>> {
>>> virDomainObjPtr obj;
>>> -virObjectLockRead(doms);
>>> +virObjectRWLockRead(doms);
>>
>> If we are going to do this (which I'm no fan of), then we should also
>> turn virObjectLock() into virObjectLockableLock(). For the consistency
>> sake. Moreover, as I stated in discussion to v1 (not sure if you've read
>> it before sending this series), I quite like that I'm able to type
>> virObjectLock, hit shortcut for bringing up completion list and chose
>> 'virObjectLock', 'virObjectLockRead', (or even) 'virObjectLockWrite'.
>> With this patch I'm no longer able to do that so easily.
>>
>> Michal
>>
>
> I read the response and the others... I'm torn between RWLockRead and
> just LockRead. I really don't care either way. I went with RW for the
> stated reason though - fear that someone like you sees virObjectLockRead
> (or worse virObjectLockWrite) and believes that is what they want.
>
> If they are not operating on an RWLockableObject, then they really don't
> get the lock and because we've decided to not error out in that case,
> they don't have the safety they thought they had.
Well, true. But aren't we trying to fix something that is no issue? I
mean, have somebody ran into such issue? But truth is I no longer care
that much.
>
> Maybe I'm wrong, but I have to present that argument.
>
> As for altering virObjectLock to virObjectLockableLock - that ship
> sailed long ago. I would say it would be virObjectMutexLock though to be
> more precise, but using virObjectLock as a shorthand since there was
> only one locking subsystem available is understandable. Time has brought
> forth some new options and now we have to adjust the new code to fit the
> more recent models. The old code could be adjusted, but there's far too
> many places that need change and no one wants that insanely impossible task.
>
> I suppose I could also see just reason to go with virObjectLockRWRdLock
> and virObjectLockRWWrLock (and virObjectUnlockRW).
>
> I haven't trained my editor to choose API names for me.
You should, esp. with such long function names :-)
>
> Not sure there's a perfect solution for this - perhaps multiple opinions
> though. I suppose all that really matters is we decide either:
>
> 1. Leave things as they are - completely
> 2. Alter the naming scheme in some way
>
> I can live with #1 even though I'm concerned about mis-use. Also, I
> thought using overloaded functions was something that long ago was
> decided to be less desirable. That is the Lock and Unlock operate on two
> different object types based on something stored in the object instead
> of two separate API's. The call is to two very different lower level
> API's as well that cannot be used interchangeably.
>
> While IIUC the goal would be to some day change all virObjectLock()'s to
> be either LockRead or LockWrite and do away with virObjectLock and any
> sort of reference to virMutexLock's and that's a noble goal. However, I
> would also think it could be awhile before that's realizable. So yes,
> it's a conundrum and I can be talked into dropping this series. Although
> I do still see value in the "magic number check" prior to using a non
> NULL @obj (a/k/a @anyobj).
I don't think that we want to replace
Re: [libvirt] [PATCH 2/1] tests: Add xml2xml tests for smartcard processing
Thanks for volunteering to fix this after me, I did not expect that,
here is my attempt:
https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html
On Thu, Aug 03, 2017 at 07:30:43AM -0400, John Ferlan wrote:
Merge into previous... With the explanation to add smartcard xml2xml
A v2 would have been easier to read
tests.
As shown below other tests already exists.
This includes the aha moment where an extra > was being printed for
smartcards using type= as a result of virDomainChrTypeFormat doing the
printing. Needed to print the > for the other two types explicitly.
Existing tests (far more exist for channel, console, serial, and parallel):
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-guestfwd.xml:
...
...
tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml:
...
...
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-pty.xml:
...
...
tests/qemuxml2xmloutdata/qemuxml2xmlout-parallel-tcp.xml:
...
...
qemuxml2xmlout-virtio-rng-egd.xml:
...
...
qemuxml2xmlout-usb-redir-filter-version.xml:
...
...
Signed-off-by: John Ferlan
---
src/conf/domain_conf.c | 3 +-
.../qemuxml2xmlout-smartcard-controller.xml| 31
.../qemuxml2xmlout-smartcard-host-certificates.xml | 34 ++
.../qemuxml2xmlout-smartcard-host.xml | 31
...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31
.../qemuxml2xmlout-smartcard-passthrough-tcp.xml | 33 +
tests/qemuxml2xmltest.c| 6
7 files changed, 168 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 878c15d..f758210 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23064,9 +23064,11 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
switch (def->type) {
case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
+virBufferAddLit(buf, ">\n");
break;
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
+virBufferAddLit(buf, ">\n");
for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
virBufferEscapeString(&childBuf, "%s\n",
def->data.cert.file[i]);
@@ -23092,7 +23094,6 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
return -1;
if (virBufferUse(&childBuf)) {
-virBufferAddLit(buf, ">\n");
This change makes the else branch pointless, since it assumes the element
will be a pair one.
I split the '>' addition out of the Chr.*Format functions completely,
to let the callers decide.
virBufferAddBuffer(buf, &childBuf);
virBufferAddLit(buf, "\n");
} else {
[...]
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index bf4d507..0d549ad 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1199,6 +1199,12 @@ mymain(void)
DO_TEST("cpu-check-default-partial", NONE);
DO_TEST("cpu-check-default-partial2", NONE);
+DO_TEST("smartcard-host", NONE);
+DO_TEST("smartcard-host-certificates", NONE);
+DO_TEST("smartcard-passthrough-tcp", NONE);
+DO_TEST("smartcard-passthrough-spicevmc", NONE);
+DO_TEST("smartcard-controller", NONE);
+
But your test cases match mine exactly :D
Jan
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix smartcard formatting regression
*** BLURB HERE *** Ján Tomko (2): tests: add smartcard test cases to qemuxml2xmltest conf: fix formatting of smartcard devices src/conf/domain_conf.c | 43 ++ .../qemuxml2xmlout-smartcard-controller.xml| 31 .../qemuxml2xmlout-smartcard-host-certificates.xml | 34 + .../qemuxml2xmlout-smartcard-host.xml | 31 ...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31 .../qemuxml2xmlout-smartcard-passthrough-tcp.xml | 33 + tests/qemuxml2xmltest.c| 6 +++ 7 files changed, 194 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml -- 2.13.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] conf: fix formatting of smartcard devices
My commit 0c1d863 broke formatting of passthrough smartcard devices:
resulted in invalid XML:
type='spicevmc'>
Split out chardev source formatting function into two -
one formatting the attributes and other formatting the subelements.
Reported-by: Cole Robinson
---
src/conf/domain_conf.c | 43 ---
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eb7052303..640f29d3e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22804,10 +22804,9 @@ virDomainNetDefFormat(virBufferPtr buf,
/* Assumes that "". */
static int
-virDomainChrSourceDefFormat(virBufferPtr buf,
-virDomainChrSourceDefPtr def,
-bool tty_compat,
-unsigned int flags)
+virDomainChrAttrsDefFormat(virBufferPtr buf,
+ virDomainChrSourceDefPtr def,
+ bool tty_compat)
{
const char *type = virDomainChrTypeToString(def->type);
@@ -22823,7 +22822,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, " tty='%s'",
def->data.file.path);
}
-virBufferAddLit(buf, ">\n");
+return 0;
+}
+
+static void
+virDomainChrSourceDefFormat(virBufferPtr buf,
+virDomainChrSourceDefPtr def,
+unsigned int flags)
+{
switch ((virDomainChrType)def->type) {
case VIR_DOMAIN_CHR_TYPE_NULL:
@@ -22924,7 +22930,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
-return 0;
+return;
}
static int
@@ -22953,8 +22959,10 @@ virDomainChrDefFormat(virBufferPtr buf,
def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
def->source->data.file.path);
-if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
+if (virDomainChrAttrsDefFormat(buf, def->source, tty_compat) < 0)
return -1;
+virBufferAddLit(buf, ">\n");
+virDomainChrSourceDefFormat(buf, def->source, flags);
/* Format block */
switch (def->deviceType) {
@@ -23067,9 +23075,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false,
-flags) < 0)
-return -1;
+virDomainChrSourceDefFormat(&childBuf, def->data.passthru, flags);
break;
default:
@@ -23083,6 +23089,10 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
return -1;
virBufferAsprintf(buf, "type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH &&
+virDomainChrAttrsDefFormat(buf, def->data.passthru, false) < 0)
+return -1;
+
if (virBufferUse(&childBuf)) {
virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &childBuf);
@@ -23390,10 +23400,11 @@ virDomainRNGDefFormat(virBufferPtr buf,
break;
case VIR_DOMAIN_RNG_BACKEND_EGD:
-virBufferAdjustIndent(buf, 2);
-if (virDomainChrSourceDefFormat(buf, def->source.chardev,
-false, flags) < 0)
+if (virDomainChrAttrsDefFormat(buf, def->source.chardev, false) < 0)
return -1;
+virBufferAddLit(buf, ">\n");
+virBufferAdjustIndent(buf, 2);
+virDomainChrSourceDefFormat(buf, def->source.chardev, flags);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "\n");
@@ -24234,9 +24245,11 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
bus = virDomainRedirdevBusTypeToString(def->bus);
virBufferAsprintf(buf, "source, false, flags) < 0)
+if (virDomainChrAttrsDefFormat(buf, def->source, false) < 0)
return -1;
+virBufferAddLit(buf, ">\n");
+virBufferAdjustIndent(buf, 2);
+virDomainChrSourceDefFormat(buf, def->source, flags);
virDomainDeviceInfoFormat(buf, &def->info,
flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
virBufferAdjustIndent(buf, -2);
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] Use a separate buffer for subelements
On 08/03/2017 08:57 AM, Ján Tomko wrote: > On Wed, Aug 02, 2017 at 01:12:15PM -0400, Cole Robinson wrote: >> On 07/26/2017 09:29 AM, Ján Tomko wrote: >>> Convert virDomainSmartcardDefFormat to use a separate buffer >>> for possible subelements, to avoid the need for duplicated >>> formatting logic in virDomainDeviceInfoNeedsFormat. >>> --- >>> src/conf/domain_conf.c | 39 +++ >>> 1 file changed, 23 insertions(+), 16 deletions(-) >>> >> >> Looks like this patch causes a regression, currently breaking the >> virt-manager >> test suite that danpb pointed out to me. Edit an existing VM and add >> >> >> >> The returned XML is invalid: >> >> >> type='spicevmc'> >> >> >> >> Unfortunately there aren't any xml2xml tests for smartcard bits that would >> have caught this... >> > > Thanks, I have sent a fix: > https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html > John sent patches too: https://www.redhat.com/archives/libvir-list/2017-August/msg00101.html - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] Use a separate buffer for subelements
On Wed, Aug 02, 2017 at 01:12:15PM -0400, Cole Robinson wrote: On 07/26/2017 09:29 AM, Ján Tomko wrote: Convert virDomainSmartcardDefFormat to use a separate buffer for possible subelements, to avoid the need for duplicated formatting logic in virDomainDeviceInfoNeedsFormat. --- src/conf/domain_conf.c | 39 +++ 1 file changed, 23 insertions(+), 16 deletions(-) Looks like this patch causes a regression, currently breaking the virt-manager test suite that danpb pointed out to me. Edit an existing VM and add The returned XML is invalid: type='spicevmc'> Unfortunately there aren't any xml2xml tests for smartcard bits that would have caught this... Thanks, I have sent a fix: https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] tests: add smartcard test cases to qemuxml2xmltest
Generated from the tests files used by qemuxml2argvtest with the
formatting function from before my commit 0c1d8632 broke smartcard
XML formatting.
---
.../qemuxml2xmlout-smartcard-controller.xml| 31
.../qemuxml2xmlout-smartcard-host-certificates.xml | 34 ++
.../qemuxml2xmlout-smartcard-host.xml | 31
...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31
.../qemuxml2xmlout-smartcard-passthrough-tcp.xml | 33 +
tests/qemuxml2xmltest.c| 6
6 files changed, 166 insertions(+)
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
new file mode 100644
index 0..dc7365b0b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
@@ -0,0 +1,31 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
new file mode 100644
index 0..e0835f6c0
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
@@ -0,0 +1,34 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+ cert1
+ cert2
+ cert3
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
new file mode 100644
index 0..abb2c4b6e
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
@@ -0,0 +1,31 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
new file mode 100644
index 0..38755e2d7
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
@@ -0,0 +1,31 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
new file mode 100644
index 0..2232daa35
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index bf4d507f6..0d549adec 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1199,6 +1199,12 @@ mymain(void)
DO_TEST("cpu-check-default-partial", NONE);
DO_TEST("cpu-check-default-partial2", NONE);
+DO_TEST("smartcard-host", NONE);
+DO_TEST("smartcard-host-certificates", NONE);
+DO_TEST("smartcard-passthrough-tcp", NONE);
+DO_TEST("smartcard-passthrough-spicevmc", NONE);
+DO_TEST("smartcard-controller", NONE);
+
if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
virFileDeleteTree(fakerootdir);
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php PATCH 02/13] Update AUTHORS file
On 08/01/2017 11:46 PM, Dawid Zamirski wrote: > Since the project is about to get a bunch of new header files, it's > much easier to maintain just the AUTHORS file. > --- > AUTHORS | 29 +++-- > src/libvirt-php.c | 10 -- > src/libvirt-php.h | 10 -- > src/sockets.c | 3 --- > src/sockets.h | 4 > src/util.c| 3 --- > src/util.h| 3 --- > src/vncfunc.c | 3 --- > src/vncfunc.h | 4 > 9 files changed, 15 insertions(+), 54 deletions(-) > > diff --git a/AUTHORS b/AUTHORS > index 9899c00..52c074d 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -3,28 +3,29 @@ Libvirt-php extension > > Libvirt-php extension is currently maintained and developed by: > > - Michal Novotny (or ) > +Michal Novotny (or ) ^^^ [1] > > The original project, called php-libvirt, has been originally developed and > maintained by: > > - Radek Hladik > +Radek Hladik > > who is still contributing to the project with his patches. > > There are also other people that have contributed to the project: > > - David King > - Jan-Paul van Burgsteden > - Lyre (or <417...@gmail.com>) > - Daniel P. Berrange > - Tiziano Mueller > - Yukihiro Kawada > -Remi Collet > -Ivo van den Abeelen > -Tiziano Müller > -Pavel Odintsov > -Tugdual Saunier > - Stefan Kuhn > +David King > +Jan-Paul van Burgsteden > +Lyre (or <417...@gmail.com>) > +Daniel P. Berrange > +Tiziano Mueller > +Yukihiro Kawada > +Remi Collet > +Ivo van den Abeelen > +Tiziano Müller > +Pavel Odintsov > +Tugdual Saunier > +Stefan Kuhn > +Dawid Zamirski You forgot my name :-) Anyway, I think it's fair to say that Michal Novotny is gone and since I'm doing the releases now I'm replacing his name with mine here [1]. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-php PATCH 04/13] Split up the bindings for libvirt connection API
On 08/01/2017 11:46 PM, Dawid Zamirski wrote: > * add libvirt-connection.h and libvirt-connection.c > * move all libvirt_connect_* function declarations and definitions to > above files > * other minor adjusments to libvirt-php.h and util.h to keep the code > compilable while the code is being moved around. > --- > src/Makefile.am | 3 +- > src/libvirt-connection.c | 885 + > src/libvirt-connection.h | 83 + > src/libvirt-php.c| 919 > +-- > src/libvirt-php.h| 104 +++--- > src/util.h | 7 - > 6 files changed, 1024 insertions(+), 977 deletions(-) > create mode 100644 src/libvirt-connection.c > create mode 100644 src/libvirt-connection.h Unfortunately, this breaks the build for me. I'm compiling with php5.6. Moreover, as I try to fix the build I wonder if we can put all the php differentiating code into util.h - it'd need to include php.h then (for all those ZEND_* macros and zend_* types). I'm pushing patches 02 and 03 meanwhile. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC]Add new mdev interface for QoS
On 2017/8/3 0:58, Alex Williamson wrote: > On Wed, 2 Aug 2017 21:16:28 +0530 > Kirti Wankhede wrote: > >> On 8/2/2017 6:29 PM, Gao, Ping A wrote: >>> On 2017/8/2 18:19, Kirti Wankhede wrote: On 8/2/2017 3:56 AM, Alex Williamson wrote: > On Tue, 1 Aug 2017 13:54:27 +0800 > "Gao, Ping A" wrote: > >> On 2017/7/28 0:00, Gao, Ping A wrote: >>> On 2017/7/27 0:43, Alex Williamson wrote: [cc +libvir-list] On Wed, 26 Jul 2017 21:16:59 +0800 "Gao, Ping A" wrote: > The vfio-mdev provide the capability to let different guest share the > same physical device through mediate sharing, as result it bring a > requirement about how to control the device sharing, we need a QoS > related interface for mdev to management virtual device resource. > > E.g. In practical use, vGPUs assigned to different quests almost has > different performance requirements, some guests may need higher > priority > for real time usage, some other may need more portion of the GPU > resource to get higher 3D performance, corresponding we can define > some > interfaces like weight/cap for overall budget control, priority for > single submission control. > > So I suggest to add some common attributes which are vendor agnostic > in > mdev core sysfs for QoS purpose. I think what you're asking for is just some standardization of a QoS attribute_group which a vendor can optionally include within the existing mdev_parent_ops.mdev_attr_groups. The mdev core will transparently enable this, but it really only provides the standard, all of the support code is left for the vendor. I'm fine with that, but of course the trouble with and sort of standardization is arriving at an agreed upon standard. Are there QoS knobs that are generic across any mdev device type? Are there others that are more specific to vGPU? Are there existing examples of this that we can steal their specification? >>> Yes, you are right, standardization QoS knobs are exactly what I wanted. >>> Only when it become a part of the mdev framework and libvirt, then QoS >>> such critical feature can be leveraged by cloud usage. HW vendor only >>> need to focus on the implementation of the corresponding QoS algorithm >>> in their back-end driver. >>> >>> Vfio-mdev framework provide the capability to share the device that lack >>> of HW virtualization support to guests, no matter the device type, >>> mediated sharing actually is a time sharing multiplex method, from this >>> point of view, QoS can be take as a generic way about how to control the >>> time assignment for virtual mdev device that occupy HW. As result we can >>> define QoS knob generic across any device type by this way. Even if HW >>> has build in with some kind of QoS support, I think it's not a problem >>> for back-end driver to convert mdev standard QoS definition to their >>> specification to reach the same performance expectation. Seems there are >>> no examples for us to follow, we need define it from scratch. >>> >>> I proposal universal QoS control interfaces like below: >>> >>> Cap: The cap limits the maximum percentage of time a mdev device can own >>> physical device. e.g. cap=60, means mdev device cannot take over 60% of >>> total physical resource. >>> >>> Weight: The weight define proportional control of the mdev device >>> resource between guests, it’s orthogonal with Cap, to target load >>> balancing. E.g. if guest 1 should take double mdev device resource >>> compare with guest 2, need set weight ratio to 2:1. >>> >>> Priority: The guest who has higher priority will get execution first, >>> target to some real time usage and speeding interactive response. >>> >>> Above QoS interfaces cover both overall budget control and single >>> submission control. I will sent out detail design later once get >>> aligned. >> Hi Alex, >> Any comments about the interface mentioned above? > Not really. > > Kirti, are there any QoS knobs that would be interesting > for NVIDIA devices? > We have different types of vGPU for different QoS factors. When mdev devices are created, its resources are allocated irrespective of which VM/userspace app is going to use that mdev device. Any parameter we add here should be tied to particular mdev device and not to the guest/app that are going to use it. 'Cap' and 'Priority' are along that line. All mdev device might not need/use these parameters, these can be made optional interfaces. >>> We also define some QoS parameters in Intel vGPU types, but it only >>> pro
Re: [libvirt] [PATCH v2] vz: support disabled items in vz boot order
On 06/30/2017 02:34 AM, Nikolay Shirokovskiy wrote:
> At the time the check was written virtuozzo did not use disabled items in boot
> order configuration. Boot items were always enabled. Now they can be disabled
> as well. Supporting such items is easy - they just should be ignored.
> ---
> src/vz/vz_sdk.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
Is this a result of commit id '8c9252aa6' - if so, then it should be
stated... If not, what changed from the original commit '032c5bf98'
that added the check is now making it unnecessary should be added to the
commit message.
Reviewed-by: John Ferlan
I won't push yet though.
John
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 8ccd7ea..a6eb0dd 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -1736,11 +1736,8 @@ prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom,
> virDomainDefPtr def)
> pret = PrlBootDev_IsInUse(bootDev, &inUse);
> prlsdkCheckRetGoto(pret, cleanup);
>
> -if (!inUse) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("Boot ordering with disabled items is not
> supported"));
> -goto cleanup;
> -}
> +if (!inUse)
> +continue;
>
> pret = PrlBootDev_GetSequenceIndex(bootDev, &bootIndex);
> prlsdkCheckRetGoto(pret, cleanup);
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/1] tests: Add xml2xml tests for smartcard processing
Merge into previous... With the explanation to add smartcard xml2xml
tests.
As shown below other tests already exists.
This includes the aha moment where an extra > was being printed for
smartcards using type= as a result of virDomainChrTypeFormat doing the
printing. Needed to print the > for the other two types explicitly.
Existing tests (far more exist for channel, console, serial, and parallel):
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-guestfwd.xml:
...
...
tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml:
...
...
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-pty.xml:
...
...
tests/qemuxml2xmloutdata/qemuxml2xmlout-parallel-tcp.xml:
...
...
qemuxml2xmlout-virtio-rng-egd.xml:
...
...
qemuxml2xmlout-usb-redir-filter-version.xml:
...
...
Signed-off-by: John Ferlan
---
src/conf/domain_conf.c | 3 +-
.../qemuxml2xmlout-smartcard-controller.xml| 31
.../qemuxml2xmlout-smartcard-host-certificates.xml | 34 ++
.../qemuxml2xmlout-smartcard-host.xml | 31
...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31
.../qemuxml2xmlout-smartcard-passthrough-tcp.xml | 33 +
tests/qemuxml2xmltest.c| 6
7 files changed, 168 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 878c15d..f758210 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23064,9 +23064,11 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
switch (def->type) {
case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
+virBufferAddLit(buf, ">\n");
break;
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
+virBufferAddLit(buf, ">\n");
for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
virBufferEscapeString(&childBuf, "%s\n",
def->data.cert.file[i]);
@@ -23092,7 +23094,6 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
return -1;
if (virBufferUse(&childBuf)) {
-virBufferAddLit(buf, ">\n");
virBufferAddBuffer(buf, &childBuf);
virBufferAddLit(buf, "\n");
} else {
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
new file mode 100644
index 000..dc7365b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
@@ -0,0 +1,31 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
new file mode 100644
index 000..e0835f6
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
@@ -0,0 +1,34 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+ cert1
+ cert2
+ cert3
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
new file mode 100644
index 000..abb2c4b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
@@ -0,0 +1,31 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-i686
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
new file mode 100644
index 000..38755e2
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
@@ -0,0 +1,31 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+
Re: [libvirt] Repack git repo?
On Thu, Aug 03, 2017 at 11:33:29AM +0200, Michal Privoznik wrote: > On 08/03/2017 09:57 AM, Daniel P. Berrange wrote: > > On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote: > >> So I was checking out the repo the other day and it took ages. So it got > >> me thinking what might be the problem. Looks like a part of it is that > >> our pack is split among ~250 files. Therefore when somebody does > >> checkout git needs to repack it into a single pack every time. And this > >> may take ages on such slow processor as Atom is. However, reading some > >> docs on this it looks like 'git gc --aggressive' is not advised rather > >> than 'git repack'. > > > > I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no > > appreciable difference. > > In fact, there is. I just finished running 'git repack -a -d' over the > 'tmp' repo and here are the results: > > $ time git clone git://libvirt.org/libvirt.git libvirt_temp.git > Cloning into 'libvirt_temp.git'... > remote: Counting objects: 236385, done. > remote: Compressing objects: 100% (38422/38422), done. > remote: Total 236385 (delta 200296), reused 232761 (delta 196975) > Receiving objects: 100% (236385/236385), 297.08 MiB | 5.55 MiB/s, done. > Resolving deltas: 100% (200296/200296), done. > > real2m40.089s > user1m2.831s > sys 0m2.970s > > $ time git clone git://libvirt.org/tmp tmp.git > Cloning into 'tmp.git'... > remote: Counting objects: 236365, done. > remote: Compressing objects: 100% (35400/35400), done. > remote: Total 236365 (delta 200277), reused 236065 (delta 199977) > Receiving objects: 100% (236365/236365), 297.19 MiB | 6.17 MiB/s, done. > Resolving deltas: 100% (200277/200277), done. > > real1m16.209s > user1m7.782s > sys 0m2.940s > > In the first case, the network transmission took ~54s, so prep work on > server took ~1m45s. In the second case, network transmission took 48s, > so prep work took just ~28s. Therefore I think it makes sense to run the > command. If nobody objects I can do that later today. Yep, that's fine with me. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repack git repo?
On 08/03/2017 09:57 AM, Daniel P. Berrange wrote: > On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote: >> So I was checking out the repo the other day and it took ages. So it got >> me thinking what might be the problem. Looks like a part of it is that >> our pack is split among ~250 files. Therefore when somebody does >> checkout git needs to repack it into a single pack every time. And this >> may take ages on such slow processor as Atom is. However, reading some >> docs on this it looks like 'git gc --aggressive' is not advised rather >> than 'git repack'. > > I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no > appreciable difference. In fact, there is. I just finished running 'git repack -a -d' over the 'tmp' repo and here are the results: $ time git clone git://libvirt.org/libvirt.git libvirt_temp.git Cloning into 'libvirt_temp.git'... remote: Counting objects: 236385, done. remote: Compressing objects: 100% (38422/38422), done. remote: Total 236385 (delta 200296), reused 232761 (delta 196975) Receiving objects: 100% (236385/236385), 297.08 MiB | 5.55 MiB/s, done. Resolving deltas: 100% (200296/200296), done. real2m40.089s user1m2.831s sys 0m2.970s $ time git clone git://libvirt.org/tmp tmp.git Cloning into 'tmp.git'... remote: Counting objects: 236365, done. remote: Compressing objects: 100% (35400/35400), done. remote: Total 236365 (delta 200277), reused 236065 (delta 199977) Receiving objects: 100% (236365/236365), 297.19 MiB | 6.17 MiB/s, done. Resolving deltas: 100% (200277/200277), done. real1m16.209s user1m7.782s sys 0m2.940s In the first case, the network transmission took ~54s, so prep work on server took ~1m45s. In the second case, network transmission took 48s, so prep work took just ~28s. Therefore I think it makes sense to run the command. If nobody objects I can do that later today. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Fix printing of 'type' and 'tty_compat' for Chr devices
On Wed, Aug 02, 2017 at 03:51:34PM -0400, John Ferlan wrote: > Commit id '0c1d8632' caused a regression in the virt-manager > test suite when formatting the type='spicevmc'/>. > > Adust the code to print the type in it's own new helper called > virDomainChrTypeFormat and have the virDomainChrSourceDefFormat > manage just formatting the source and change to a void type since > only 0 could be returned. Adjust the callers to handle properly. > > Signed-off-by: John Ferlan > --- > > Although technically a CI build breaker since virt-manager test is > failing, I figured I'd let this one go through the formal review just > in case someone has agita over new function name or would like to see > things done in a different manner. > > > src/conf/domain_conf.c | 39 --- > 1 file changed, 24 insertions(+), 15 deletions(-) Can you add a test case for that, since the lack of tests is why we have this regression in tree, and I'd feel comfortable that it actually fixes the problem if tests show it. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repack git repo?
On Thu, Aug 03, 2017 at 10:33:47AM +0200, Peter Krempa wrote: On Thu, Aug 03, 2017 at 09:16:13 +0200, Michal Privoznik wrote: So I was checking out the repo the other day and it took ages. So it got me thinking what might be the problem. Looks like a part of it is that our pack is split among ~250 files. Therefore when somebody does 250 files should not be an issue on today's computers. Maybe 2 orders of magnitude more would pose some issues. checkout git needs to repack it into a single pack every time. And this may take ages on such slow processor as Atom is. However, reading some docs on this it looks like 'git gc --aggressive' is not advised rather than 'git repack'. Any thoughts? Clone from github or some other hosting with more network conectivity/CPU power if you are bothered by this? Yes, you can clone from github and then switch origins, sure. I was just interested how stuff is affected and should be handled =) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent
On Thu, Aug 03, 2017 at 10:28:59 +0200, Bjoern Walk wrote: > Peter Krempa [2017-08-03, 09:47AM +0200]: > > On Thu, Aug 03, 2017 at 07:24:35 +0200, Bjoern Walk wrote: > > > Peter Krempa [2017-08-02, 05:39PM +0200]: > > > > It turns out that our implementation of the hashing function is > > > > endian-dependent and thus if used on various architectures the testsuite > > > > may have different results. Work this around by mocking virHashCodeGen > > > > to something which does not use bit operations instead of just setting a > > > > deterministic seed. > > > > > > This does fix the issue on S390. Anyways, I'd also like to see the tests > > > fixed that rely on the ordering of the hash table. And code that uses > > > > The tests are fixed. They are ordered correctly to the newly mocked > > function. I don't quite get what more you'd like to see fixed. > > > > No, the test is still dependent on the ordering. Just now the mocked > hash table has deterministic ordering. This is not the same. Although it > is improbable that this will bite us somewhere, I'd still prefer a clean > solution. I don't think it's worth doing this in the tests. The hash table at least in case of the node name detection code is just an intermediate step to fill the data into the domain definition and it's a conveniet place to test the detection code. Testing it in other places would remove the dependency on deterministic ordering of the hash table but would not really add much value. The cost would be much more complexity in the test case which I don't think it's worth. If you feel bothered by it, please post patches. I think currently the testsuite tests the complex portion of the code without the bloating necessary for the so-called "clean" solution. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repack git repo?
On Thu, Aug 03, 2017 at 09:16:13 +0200, Michal Privoznik wrote: > So I was checking out the repo the other day and it took ages. So it got > me thinking what might be the problem. Looks like a part of it is that > our pack is split among ~250 files. Therefore when somebody does 250 files should not be an issue on today's computers. Maybe 2 orders of magnitude more would pose some issues. > checkout git needs to repack it into a single pack every time. And this > may take ages on such slow processor as Atom is. However, reading some > docs on this it looks like 'git gc --aggressive' is not advised rather > than 'git repack'. > > Any thoughts? Clone from github or some other hosting with more network conectivity/CPU power if you are bothered by this? signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent
Peter Krempa [2017-08-03, 09:47AM +0200]: > On Thu, Aug 03, 2017 at 07:24:35 +0200, Bjoern Walk wrote: > > Peter Krempa [2017-08-02, 05:39PM +0200]: > > > It turns out that our implementation of the hashing function is > > > endian-dependent and thus if used on various architectures the testsuite > > > may have different results. Work this around by mocking virHashCodeGen > > > to something which does not use bit operations instead of just setting a > > > deterministic seed. > > > > This does fix the issue on S390. Anyways, I'd also like to see the tests > > fixed that rely on the ordering of the hash table. And code that uses > > The tests are fixed. They are ordered correctly to the newly mocked > function. I don't quite get what more you'd like to see fixed. > No, the test is still dependent on the ordering. Just now the mocked hash table has deterministic ordering. This is not the same. Although it is improbable that this will bite us somewhere, I'd still prefer a clean solution. > > the hash table should be tested that it does NOT rely on the ordering. > > Well, that's a property of the hash table that the code should not > depend on ordering. Yes, and I think it would be a good idea to test for this (no idea how to do this though). > In fact the only part which is slightly dependant on ordering is the > test suite. The fix to mock the ordering function properly is tested. > Like with all mocks, we now test a different version of code in the test suite. We test the code under the assumption that the hash table is ordered deterministically. However, the real code doesn't fulfill this assumption (rightly so, because ordering is not a property of the hash table). There's a discrepancy. For example, let's assume we have code that writes out an XML file from a hash table. We do this similar to the test code in testBlockNodeNameDetect. Writing a test against this will never fail, because we test against the mocked hash table. However, the actual code produces results that is dependent on the ordering. I know this is nitpicky, but if someone down the road runs into any problems with this it is nasty do understand. > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- IBM Systems Linux on z Systems & Virtualization Development IBM Deutschland Schönaicher Str. 220 71032 Böblingen Phone: +49 7031 16 1819 E-Mail: bw...@de.ibm.com IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: Add "PCI topology and hotplug" guidelines
On Wed, 2017-08-02 at 11:06 +0200, Marcin Juszkiewicz wrote: > Thanks a lot for that documentation. Glad you found it useful :) Hopefully I can get it merged soon. > At Linaro we use AArch64 servers to deploy VM machines (with OpenStack > Newton or just libvirt) and issues with PCI hotplug came again and again. > > https://bugs.linaro.org/show_bug.cgi?id=2819 is main one. It was created > when we used libvirt 2.2.0 but now we are at 3.4.0 version and problem > is finally debugged properly ;D > > From my tests it looks like adding 9-10 pcie-root-port devices will > handle our use - just have to convince OpenStack guys about it (or find > a way around libvirt to get those by default). The number of PCIe Root Ports provided by libvirt by default is very unlikely to change: the current behavior, which is consistent among aarch64/virt and x86_64/q35, has been chosen specifically because it provides enough flexibility for casual users without enforcing (too much of) a policy, which is something that libvirt actively tries to avoid. Adding extra PCIe Root Ports as appropriate for the specific guest flavor is left as an exercise for the higher-level management tool, in your case OpenStack. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repack git repo?
On Thu, Aug 03, 2017 at 10:14:02 +0200, Martin Kletzander wrote: > On Thu, Aug 03, 2017 at 08:57:47AM +0100, Daniel P. Berrange wrote: > >On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote: > >> So I was checking out the repo the other day and it took ages. So it got > >> me thinking what might be the problem. Looks like a part of it is that > >> our pack is split among ~250 files. Therefore when somebody does > >> checkout git needs to repack it into a single pack every time. And this > >> may take ages on such slow processor as Atom is. However, reading some > >> docs on this it looks like 'git gc --aggressive' is not advised rather > >> than 'git repack'. > > > >I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no > >appreciable difference. > > > > Other thought I had was having the po/*.po files removed from the repo. > DV would add them to the release, but they would not take 100MB in the > repository when they are not needed for development builds. It would > not cut down on the data that need to be transferred since they are in > the history, but at least would take some weight off the repo. After > few years we could do a split as linux did and have one repo with > complete history and one that goes from v4.0.0 onwards. NACK. There's no real benefit in crippling the repo like this. You can always make a shallow clone of the repo if you don't want to get the complete history. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repack git repo?
On Thu, Aug 03, 2017 at 08:57:47AM +0100, Daniel P. Berrange wrote: On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote: So I was checking out the repo the other day and it took ages. So it got me thinking what might be the problem. Looks like a part of it is that our pack is split among ~250 files. Therefore when somebody does checkout git needs to repack it into a single pack every time. And this may take ages on such slow processor as Atom is. However, reading some docs on this it looks like 'git gc --aggressive' is not advised rather than 'git repack'. I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no appreciable difference. Other thought I had was having the po/*.po files removed from the repo. DV would add them to the release, but they would not take 100MB in the repository when they are not needed for development builds. It would not cut down on the data that need to be transferred since they are in the history, but at least would take some weight off the repo. After few years we could do a split as linux did and have one repo with complete history and one that goes from v4.0.0 onwards. Just my $.02 Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Repack git repo?
On Thu, Aug 03, 2017 at 09:16:13AM +0200, Michal Privoznik wrote: > So I was checking out the repo the other day and it took ages. So it got > me thinking what might be the problem. Looks like a part of it is that > our pack is split among ~250 files. Therefore when somebody does > checkout git needs to repack it into a single pack every time. And this > may take ages on such slow processor as Atom is. However, reading some > docs on this it looks like 'git gc --aggressive' is not advised rather > than 'git repack'. I created a 'tmp' repo and ran 'repack' on it, but afaict, there's no appreciable difference. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent
On Thu, Aug 03, 2017 at 07:24:35 +0200, Bjoern Walk wrote: > Peter Krempa [2017-08-02, 05:39PM +0200]: > > It turns out that our implementation of the hashing function is > > endian-dependent and thus if used on various architectures the testsuite > > may have different results. Work this around by mocking virHashCodeGen > > to something which does not use bit operations instead of just setting a > > deterministic seed. > > This does fix the issue on S390. Anyways, I'd also like to see the tests > fixed that rely on the ordering of the hash table. And code that uses The tests are fixed. They are ordered correctly to the newly mocked function. I don't quite get what more you'd like to see fixed. > the hash table should be tested that it does NOT rely on the ordering. Well, that's a property of the hash table that the code should not depend on ordering. In fact the only part which is slightly dependant on ordering is the test suite. The fix to mock the ordering function properly is tested. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Honour
https://bugzilla.redhat.com/show_bug.cgi?id=1476866
For some reason, we completely ignore setting for
domains. The implementation is simply not there. It never was.
However, things are slightly more complicated. QEMU sends us two
RESET events on domain reboot. Fortunately, the event contains
this 'guest' field telling us who initiated the reboot. And since
we don't want to destroy the domain if the reset is initiated by
a user, we have to ignore those events. Whatever, just look at
the code.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 8 +++-
src/qemu/qemu_process.c | 34 ++
5 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 4c9050aff..d865e67c7 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -233,6 +233,7 @@ struct _qemuDomainObjPrivate {
bool agentError;
bool gotShutdown;
+bool gotReset;
bool beingDestroyed;
char *pidfile;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19082d8bf..8f81a2b28 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1344,12 +1344,12 @@ qemuMonitorEmitShutdown(qemuMonitorPtr mon,
virTristateBool guest)
int
-qemuMonitorEmitReset(qemuMonitorPtr mon)
+qemuMonitorEmitReset(qemuMonitorPtr mon, virTristateBool guest)
{
int ret = -1;
VIR_DEBUG("mon=%p", mon);
-QEMU_MONITOR_CALLBACK(mon, ret, domainReset, mon->vm);
+QEMU_MONITOR_CALLBACK(mon, ret, domainReset, mon->vm, guest);
return ret;
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31f7e97ba..8c33f6783 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -134,6 +134,7 @@ typedef int
(*qemuMonitorDomainShutdownCallback)(qemuMonitorPtr mon,
void *opaque);
typedef int (*qemuMonitorDomainResetCallback)(qemuMonitorPtr mon,
virDomainObjPtr vm,
+ virTristateBool guest,
void *opaque);
typedef int (*qemuMonitorDomainPowerdownCallback)(qemuMonitorPtr mon,
virDomainObjPtr vm,
@@ -346,7 +347,7 @@ int qemuMonitorEmitEvent(qemuMonitorPtr mon, const char
*event,
long long seconds, unsigned int micros,
const char *details);
int qemuMonitorEmitShutdown(qemuMonitorPtr mon, virTristateBool guest);
-int qemuMonitorEmitReset(qemuMonitorPtr mon);
+int qemuMonitorEmitReset(qemuMonitorPtr mon, virTristateBool guest);
int qemuMonitorEmitPowerdown(qemuMonitorPtr mon);
int qemuMonitorEmitStop(qemuMonitorPtr mon);
int qemuMonitorEmitResume(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index b8a68154a..8a1501ced 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -536,7 +536,13 @@ static void qemuMonitorJSONHandleShutdown(qemuMonitorPtr
mon, virJSONValuePtr da
static void qemuMonitorJSONHandleReset(qemuMonitorPtr mon, virJSONValuePtr
data ATTRIBUTE_UNUSED)
{
-qemuMonitorEmitReset(mon);
+bool guest = false;
+virTristateBool guest_initiated = VIR_TRISTATE_BOOL_ABSENT;
+
+if (data && virJSONValueObjectGetBoolean(data, "guest", &guest) == 0)
+guest_initiated = guest ? VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
+
+qemuMonitorEmitReset(mon, guest_initiated);
}
static void qemuMonitorJSONHandlePowerdown(qemuMonitorPtr mon, virJSONValuePtr
data ATTRIBUTE_UNUSED)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0aecce3b1..889efc7f0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -478,27 +478,51 @@ qemuProcessFindVolumeQcowPassphrase(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
static int
qemuProcessHandleReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virDomainObjPtr vm,
+ virTristateBool guest_initiated,
void *opaque)
{
virQEMUDriverPtr driver = opaque;
-virObjectEventPtr event;
+virObjectEventPtr event = NULL;
qemuDomainObjPrivatePtr priv;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+bool callOnReboot = false;
virObjectLock(vm);
+priv = vm->privateData;
+
+/* This is a bit tricky. When a guest does 'reboot' we receive RESET event
+ * twice, both times it's guest initiated. However, if users call 'virsh
+ * reset' we still receive two events but the first one is guest_initiated
+ * = no, the second one is guest_initiated = yes. Therefore, to avoid
+ * executing onReboot action in the latter case we need this complicated
+ * construction. */
+if (guest_initiated == VIR
[libvirt] Repack git repo?
So I was checking out the repo the other day and it took ages. So it got me thinking what might be the problem. Looks like a part of it is that our pack is split among ~250 files. Therefore when somebody does checkout git needs to repack it into a single pack every time. And this may take ages on such slow processor as Atom is. However, reading some docs on this it looks like 'git gc --aggressive' is not advised rather than 'git repack'. Any thoughts? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] support for configuring all ports of a multiport SRIOV VF when assigning to guest
Another think I forgot is that driver should be configured with probe_vf [1] to work in a single port vf for ConnectX-3/PRO. For example if my driver configuration is as follows: options mlx4_core port_type_array=2,2 num_vfs=4,4,0 probe_vf=4,4,0 I will have 4 VF on port 1 and 4 VF on port 2, but ip link show you will see each PF with 8 VF. VF 0-3 will be used for the first PF (enp6s0 ) and VF 4-7 will be used for the second PF (enp6s0d1) 15: enp6s0: mtu 1500 qdisc mq master ovs-system state UP mode DEFAULT group default qlen 1000 link/ether 00:02:c9:e9:c2:11 brd ff:ff:ff:ff:ff:ff vf 0 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 1 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 2 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 3 MAC fa:16:3e:c4:61:14, vlan 128, spoof checking on, link-state enable vf 4 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 5 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 6 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 7 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto 16: enp6s0d1: mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000 link/ether 00:02:c9:e9:c2:12 brd ff:ff:ff:ff:ff:ff vf 0 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 1 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 2 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 3 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 4 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 5 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto vf 6 MAC fa:16:3e:e2:93:37, vlan 3, spoof checking off, link-state auto vf 7 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off, link-state auto [1] - https://community.mellanox.com/docs/DOC-1484#jive_content_id_Configuring_8_VFs_on_a_dual_port_NIC_while_4_VFs_are_probed_on_port_1_and_4_VFs_are_probed_on_port_2 -Original Message- From: Moshe Levi Sent: Thursday, August 3, 2017 9:34 AM To: Laine Stump ; Libvirt Cc: Doug Ledford ; Daniel P. Berrange Subject: RE: support for configuring all ports of a multiport SRIOV VF when assigning to guest Hi Laine, I have a few question before I can give my opinion. I the Mellanox Card Dual Port that support one PCI with 2 PF is ConnectX-3 and ConnectX-3 Pro. (maybe others cards I will check this) The ConnectX-4 Dual Port and above is implemented with 2 PCI devices per 2 PF. I can check with our driver architect why it was done like this in the past. The pci address in the xml below is VF pci which is different between all VF so I am not sure why it causing problems with libvirt for setting mac? I will do a little shift to openstack with SR-IOV mechanism driver. I remember with try to enable support on the second port for such cards in openstack I remember we tested Mellanox ConnectX-3 Pro Dual Port with openstack to allow boot a vm on both ports. I implemented the pci-passthrough-whitelist-regex to allow a flexibly way to whitelist pci device. And we also had a patch in neutron for the SR-IOV to allow the agent to all
Re: [libvirt] support for configuring all ports of a multiport SRIOV VF when assigning to guest
Hi Laine,
I have a few question before I can give my opinion.
I the Mellanox Card Dual Port that support one PCI with 2 PF is ConnectX-3 and
ConnectX-3 Pro. (maybe others cards I will check this)
The ConnectX-4 Dual Port and above is implemented with 2 PCI devices per 2 PF.
I can check with our driver architect why it was done like this in the past.
The pci address in the xml below is VF pci which is different between all VF so
I am not sure why it causing problems with libvirt for setting mac?
I will do a little shift to openstack with SR-IOV mechanism driver.
I remember with try to enable support on the second port for such cards in
openstack
I remember we tested Mellanox ConnectX-3 Pro Dual Port with openstack to
allow boot a vm on both ports.
I implemented the pci-passthrough-whitelist-regex to allow a flexibly way to
whitelist pci device.
And we also had a patch in neutron for the SR-IOV to allow the agent to allow
mapping of multiple PFs to a PCI device, but the community didn't like it
especially intel.
[1] -
https://specs.openstack.org/openstack/nova-specs/specs/liberty/approved/pci-passthrough-whitelist-regex.html
[2] - https://review.openstack.org/#/c/409526/
-Original Message-
From: sendmail [mailto:justsendmailnothinge...@gmail.com] On Behalf Of Laine
Stump
Sent: Thursday, August 3, 2017 7:09 AM
To: Libvirt
Cc: Doug Ledford ; Moshe Levi ;
Daniel P. Berrange
Subject: RFC: support for configuring all ports of a multiport SRIOV VF when
assigning to guest
("No matter how far you've gone down the wrong road, turn back." - paraphrase
of a Turkish proverb that is apropos to this discussion)
Several years ago, when I was apparently naive and narrow in my thinking and
someone wanted us support setting the MAC address and vlan tag for SRIOV VFs
when assigning them to a guest with PCI device assignment (this was before VFIO
existed), I had the idea to do this by creating a new type of
device:
My thinking was that already had elements for mac address,
802.11Qb[gh] virtualport config, and vlan tag (or maybe it was that we were
*going to add* support for vlan tag), so by just adding a that was a
PCI address, we would have everything we needed. Basically, there is some
amount of config that needs to be applied to the device before it's assigned to
the guest, and since the device ends up being a netdev in the guest, all that
config is already present in an . As a bonus, because it was an
we could easily re-use the recently added "pool of devices" network
type (with some minor adjustment) to avoid needing to hardcode the host-side
PCI address of the VF.
At the time Dan Berrange countered (I think - correct me if I'm wrong!) that we
should instead do this with modifications to , but somehow I managed
to either convince him, or maybe he just finally tired of my stubbornness and
decided it was easier to deal with the after effects of giving in rather than
continuing to debate with me :-)
So right now if you want to assign an SRIOV VF network device to a guest with
VFIO, you need something like this (ignoring network device pools for the
moment):
(or in place of , you could have a element for
802.11Qb[gh]).
The SRIOV cards that we had around when we were doing this work had multiple
physical ports on them (either 2 or 4), but each physical port was associated
with its own PCI Physical Function (PF), and each of the PCI Virtual Functions
associated with a PF was tied to a single netdev, i.e. in all cases there was
always a 1:1 correspondence between a netdev and a PCI device. All of libvirt's
code dealing with SRIOV VFs and PFs assumes this 1:1 relationship.
And then came Mellanox "dual port" SRIOV cards
A Mellanox SRIOV NIC doesn't necessarily do that. Instead, it can operate in
"dual port" mode, where it has a single PCI PF device for both physical ports;
the single PF PCI device has 2 separate netdevs associated with it (so when you
look in the "net" subdirectory for the PCI device, you'll see two netdevs
listed, and when you look in the "device" subdirectory of those two netdevs in
sysfs, they both point back to the same PCI device). VFs associated with that
PF will also each have two netdevs associated with them. This means that when
you assign a VF to a guest, the guest is getting a single PCI device, but it's
getting two netdevs. (I've been told that the advantage of doing both ports
with a single PCI device is that each Mellanox PCI device uses a huge amount of
MMIO space, two ports on each device cuts the MMIO usage in half).
In order for this to be useful, libvirt needs to set the mac address and vlan
tag of *both* netdevs prior to starting the guest. But we have no way to
represent that in our configuration. In the past it's been suggested that we
just do something like this:
...
but I ha
