[libvirt] [PATCH] Properly ignore files in build-aux directory
We want to ignore all files except *.pl in build-aux directory, however the unignore pattern "!/build-aux/*.pl" doesn't have any effect because a previous "/build-aux/" pattern ignores the directory itself rather than individual files in it. https://bugzilla.redhat.com/show_bug.cgi?id=1439994 Signed-off-by: Jiri Denemark --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 7b71bd159..d7927e1d4 100644 --- a/.gitignore +++ b/.gitignore @@ -43,7 +43,7 @@ /NEWS /aclocal.m4 /autom4te.cache -/build-aux/ +/build-aux/* /build/ /confdefs.h /config.cache -- 2.12.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-3.2] NBD-based storage migration fails with "error: invalid argument: monitor must not be NULL"
On Thu, Apr 06, 2017 at 18:14:07 +0200, Kashyap Chamarthy wrote: > [Filed this bug -- https://bugzilla.redhat.com/show_bug.cgi?id=1439841] > > Easy reproducer: > > $ virsh migrate --verbose --copy-storage-all \ > --p2p --live l2-f25 qemu+ssh://root@devstack-a/system > error: invalid argument: monitor must not be NULL This is caused by the TLS migration code and most likely fixed by https://www.redhat.com/archives/libvir-list/2017-April/msg00219.html Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 9:04 PM, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 01:25:35PM +0100, Daniel P. Berrange wrote: > > On Thu, Apr 06, 2017 at 08:20:56PM +0800, Eli Qiao wrote: > > > This patch is based on Martin's cache branch. > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > > > > > > > > > > > Why do we need to report 'type' on both bank & control elements. Are they > > really expected to have different values ? > > > > > > There’s a discussion from https://www.redhat.com/archives/libvir-list/2017-March/msg01689.html I think I made a mistake here, it should be ’scope’ instead of ’type’ here. No matter what’s it should be, at least we need to identify if the host enabled CDP for L3 cache or not. If enabled CDP on host, the resctrl looks like this: root@s2600wt:/sys/fs/resctrl# cat schemata L3DATA:0=f;1=f L3CODE:0=f;1=f root@s2600wt:/sys/fs/resctrl# cat info/L3 L3CODE/ L3DATA/ root@s2600wt:/sys/fs/resctrl# cat info/L3DATA/num_closids 8 root@s2600wt:/sys/fs/resctrl# cat info/L3CODE/num_closids 8 as you can see that num_closids are divided into 2 part (compared to 16 without CDP), but other info are still same root@s2600wt:/sys/fs/resctrl# cat info/L3CODE/cbm_mask f root@s2600wt:/sys/fs/resctrl# cat info/L3CODE/min_cbm_bits 1 root@s2600wt:/sys/fs/resctrl# cat info/L3DATA/cbm_mask f root@s2600wt:/sys/fs/resctrl# cat info/L3DATA/min_cbm_bits 1 -- below output are from disable CDP: root@s2600wt:/sys/fs/resctrl# cat schemata L3:0=f;1=f root@s2600wt:/sys/fs/resctrl# cat info/L3/ cbm_mask min_cbm_bits num_closids root@s2600wt:/sys/fs/resctrl# cat info/L3/cbm_mask f root@s2600wt:/sys/fs/resctrl# cat info/L3/min_cbm_bits 1 root@s2600wt:/sys/fs/resctrl# cat info/L3/num_closids 16 > > > > > That's one of my questions I had way back in some of the previous > discussions. Did not get the answer. I suspect there is a reason to > whether CDP is enabled or not and it is not just the type of the cache > bank itself. If it is, then CDP makes no sense for us at all, actually > for anyone who has access to cache information and that would mean bad > design of that thing. And that's not something I would expect from this > functionality. > > > > > Regards, > > Daniel > > -- > > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > > |: http://libvirt.org -o- http://virt-manager.org :| > > |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > > > > > -- > libvir-list mailing list > libvir-list@redhat.com (mailto:libvir-list@redhat.com) > https://www.redhat.com/mailman/listinfo/libvir-list > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 8:46 PM, Martin Kletzander wrote:
> On Thu, Apr 06, 2017 at 08:20:56PM +0800, Eli Qiao wrote:
> > This patch is based on Martin's cache branch.
> >
> > This patch amends the cache bank capability as follow:
>
> It helps a lot if you wait for a conclusion on a patch before sending
> another version as soon as you can change one line.
>
Okay, I will keep my patience next time, this time is just because I was
working over time yesterday.
>
> >
> >
> >
> >
> >
> >
> >
> >
> >
>
>
> I know Dan proposed "nallocations", but it sounds like one word. I
> would rather use "allocations" or "max_allocs" or something
> understandable. The reason for it? We have no documentation for our
> capabilities XML. And nobody is trying to create one as far as I know.
> So at least the naming should be more intuitive.
>
yep, I will wait for the final decision.
>
> > Along with vircaps2xmltest case updated.
>
>
> I'm sensing you haven't ran the tests. Am I right?
>
Why ?
taget@s2600wt:~/libvirt$ ./tests/vircaps2xmltest
TEST: vircaps2xmltest
4 OK
>
> > Signed-off-by: Eli Qiao > (mailto:liyong.q...@intel.com)>
> > ---
> > src/conf/capabilities.c | 120 ++-
> > src/conf/capabilities.h | 13 ++-
> > tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
> > tests/vircaps2xmltest.c | 13 +++
> > 4 files changed, 148 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index 416dd1a..3094e48 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -52,6 +52,7 @@
> > #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
> >
> > #define SYSFS_SYSTEM_PATH "/sys/devices/system/"
> > +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
> >
> > VIR_LOG_INIT("conf.capabilities")
> >
> > @@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > virCapsHostCacheBankPtr *caches)
> > {
> > size_t i = 0;
> > + size_t j = 0;
> >
> > if (!ncaches)
> > return 0;
> > @@ -894,13 +896,32 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > */
> > virBufferAsprintf(buf,
> > " > - "size='%llu' unit='%s' cpus='%s'/>\n",
> > + "size='%llu' unit='%s' cpus='%s'",
> > bank->id, bank->level,
> > virCacheTypeToString(bank->type),
> > bank->size >> (kilos * 10),
> > kilos ? "KiB" : "B",
> > cpus_str);
> >
> > + if (bank->ncontrol > 0) {
> > + virBufferAddLit(buf, ">\n");
> > + virBufferAdjustIndent(buf, 2);
> > + for (j = 0; j < bank->ncontrol; j++) {
> > + bool min_kilos = !(bank->controls[j]->min % 1024);
> > + virBufferAsprintf(buf,
> > + " > + "type='%s' nallocations='%u'/>\n",
> > + bank->controls[j]->min >> (min_kilos * 10),
> > + min_kilos ? "KiB" : "B",
> > + virCacheTypeToString(bank->controls[j]->type),
> > + bank->controls[j]->nallocations);
> > + }
> > + virBufferAdjustIndent(buf, -2);
> > + virBufferAddLit(buf, "\n");
> > + } else {
> > + virBufferAddLit(buf, "/>\n");
> > + }
> > +
> >
>
>
> There's virBufferAddBuffer() for easier usage of this. Just grep for
> childrenBuf and you'll see the usage.
>
Good to know that , thx.
>
> > VIR_FREE(cpus_str);
> > }
> >
> > @@ -1513,13 +1534,97 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr
> > a,
> > void
> > virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> > {
> > + size_t i;
> > +
> > if (!ptr)
> > return;
> >
> > virBitmapFree(ptr->cpus);
> > + for (i = 0; i < ptr->ncontrol; i++)
> > + VIR_FREE(ptr->controls[i]);
> > + VIR_FREE(ptr->controls);
> > VIR_FREE(ptr);
> > }
> >
> > +/* test which kinds of cache control supported
> > + * -1: don't support
> > + * 0: cat
> > + * 1: cdp
> > + */
> > +static int
> > +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> > +{
> > + int ret = -1;
> > + char *path = NULL;
> > + if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
> > + return -1;
> > +
> > + if (virFileExists(path)) {
> > + ret = 0;
> > + } else {
> > + VIR_FREE(path);
> > + if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level) <
> > 0)
> > + return -1;
> > + if (virFileExists(path))
> > + ret = 1;
> > + }
> > +
> > + VIR_FREE(path);
> > + return ret;
> > +}
> > +
> > +static int
> > +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char*
> > type)
> > +{
> > + int ret = -1;
> > + char *path = NULL;
> > + char *cbm_mask = NULL;
> > + virCapsHostCacheControlPtr control;
> > +
> > + if (VIR_ALLOC(control) < 0)
> > + goto cleanup;
> > +
> > + if (virFileReadValueUint(&control->nallocations,
> > + SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
> > + bank->level,
> > + type) < 0)
> > + goto cleanup;
> > +
> > + if (virFileReadValueString(&cbm_mask,
> > + SYSFS_RESCTRL_PATH
> > + "info/L%u%s/cbm_mask",
> > + bank->level,
> > + type) < 0)
> > + goto cleanup;
> > +
> > + virStringTrimOptionalNewline(cbm_mask);
> > +
> > + control->min = bank->size / (strlen(cbm_mask) * 4);
> > +
> > + if (STR
Re: [libvirt] [PATCH v3 0/7] Provide an standard asyncio event loop impl
On Tue, Apr 04, 2017 at 03:31:27PM +0100, Daniel P. Berrange wrote:
> $ python3 ./examples/event-test.py --loop=asyncio --timeout=30 qemu:///session
Thank you for this update. I tested it backported to 3.1.0 with xen:/// using
event-test.py and also with our own code. Looks good to me.
I encountered one small problem, which I believe is orthogonal to the patch
series:
On Tue, Apr 04, 2017 at 03:31:32PM +0100, Daniel P. Berrange wrote:
> diff --git a/examples/event-test.py b/examples/event-test.py
> index 751a140..ac9fbe1 100755
> --- a/examples/event-test.py
> +++ b/examples/event-test.py
> +netcallbacks = []
> +netcallbacks.append(vc.networkEventRegisterAny(None,
> libvirt.VIR_NETWORK_EVENT_ID_LIFECYCLE, myNetworkEventLifecycleCallback,
> None))
With vc = libvirt.open('xen:///') and with libvirt{,-python}-3.1.0 this line
causes an
exception:
libvirt.libvirtError: this function is not supported by the connection
driver: virConnectNetworkEventRegisterAny
Commenting it out solves the problem.
--
pozdrawiam / best regards _.-._
Wojtek Porczyk .-^' '^-.
Invisible Things Lab |'-.-^-.-'|
| | | |
I do not fear computers,| '-.-' |
I fear lack of them.'-._ : ,-'
-- Isaac Asimov `^-^-_>
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-3.2] NBD-based storage migration fails with "error: invalid argument: monitor must not be NULL"
[Filed this bug -- https://bugzilla.redhat.com/show_bug.cgi?id=1439841] Easy reproducer: $ virsh migrate --verbose --copy-storage-all \ --p2p --live l2-f25 qemu+ssh://root@devstack-a/system error: invalid argument: monitor must not be NULL - - - This is a nested Fedora-25 environment. So, I'm trying to migrate a level-2 Fedora-25 guest. libvirt version: $ rpm -q libvirt-daemon-kvm libvirt-daemon-kvm-3.2.0-1.fc25.x86_64 The QEMU binary used to boot the nested guest (to be migrated) on source guest hypervisor (host from which migration originates) _and_ on the destination guest hypervisor is built from Git, with this specific patch from QEMU https://lists.nongnu.org/archive/html/qemu-block/2017-04/msg00085.html -- [RFC PATCH for-2.9] block: Ignore guest dev permissions during incoming migration So the built binary version (showing `git describe` output): $ /home/stack/build/build-qemu/x86_64-softmmu/qemu-system-x86_64 --version QEMU emulator version 2.8.93 (v2.9.0-rc3-3-g3a8624b) NB: However, the QEMU version is irrelevant, Michal Privoznik confirms on IRC that this is a clear libvirt bug. Steps to reproduce -- (1) Setup two hosts: https://kashyapc.fedorapeople.org/virt/libvirt-migration-tests-with-qemu+tcp.txt (2) Then, migrate the guest to the destination host: $ virsh migrate --verbose --copy-storage-all \ --p2p --live l2-f25 qemu+ssh://root@devstack-a/system Actual results -- $ virsh migrate --verbose --copy-storage-all \ --p2p --live l2-f25 qemu+ssh://root@devstack-a/system error: invalid argument: monitor must not be NULL -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH (RFC?)] Remove usage of __attribute__((nonnull))
On Thu, Apr 06, 2017 at 05:40:09PM +0200, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 04:27:25PM +0200, Andrea Bolognani wrote: > > On Wed, 2017-04-05 at 06:53 -0400, John Ferlan wrote: > > > > I thought that John still had a nightly coverity job running that > > > > would trigger the -DSTATIC_ANALYSIS codepaths. If that's not the case, > > > > then we'd wnat to look > > > > at enabling that in one of the centos CI jobs. > > > > > > I still have a run of Coverity every night although I have been less > > > diligent about checking errors lately. Mainly because generally things > > > that are considered a false positive were being rejected when I posted > > > patches. I keep about 20 patches in a local branch. > > > > > > There was a point quite a few months ago where my nightly build started > > > failing because either I changed the Coverity version or the compiler > > > version on my laptop - cannot recall exactly. That resulted in a bunch > > > more local patches until I finally had too many and posted that pile > > > late last month. Enabling static analysis in CI builds was something I > > > suggested in my cover - although now I've done it for my every day work > > > environment so I will see the problems much sooner. > > > > Did we ever get in touch with the Coverity folks about > > enabling Coverity Scan[1] for libvirt? QEMU is already > > part of the program. > > > > No idea, but we can register there ourselves, can't we? They have > integration with GitHub where we have the read-only copy anyway, we can > even run in on TravisCI and have the whole bundle without changing > much internally (just committing settings file into the repository). Yep, I'd be in favour of using the public coverity scan service. For the sake of community inclusivity / transparency, it is always preferrable to have project infrastructure visible to the full commnunity where possible. We could certainly setup a travis CI build too. In particular that would give us build testing on several versions of Ubuntu, as well as ability to do OS-X builds which is a notable platform we lack any automated testing of. > I think we would get 2 builds per day, depending on how the github repo > is updated. Is that done automatically or every X minutes or something? We only do one build a day right now, so the 2 builds a day limt is no big deal. Their FAQ is fuzzy on just how builds a triggered... Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH (RFC?)] Remove usage of __attribute__((nonnull))
On Thu, Apr 06, 2017 at 04:27:25PM +0200, Andrea Bolognani wrote: On Wed, 2017-04-05 at 06:53 -0400, John Ferlan wrote: > I thought that John still had a nightly coverity job running that > would trigger the -DSTATIC_ANALYSIS codepaths. If that's not the case, then we'd wnat to look > at enabling that in one of the centos CI jobs. I still have a run of Coverity every night although I have been less diligent about checking errors lately. Mainly because generally things that are considered a false positive were being rejected when I posted patches. I keep about 20 patches in a local branch. There was a point quite a few months ago where my nightly build started failing because either I changed the Coverity version or the compiler version on my laptop - cannot recall exactly. That resulted in a bunch more local patches until I finally had too many and posted that pile late last month. Enabling static analysis in CI builds was something I suggested in my cover - although now I've done it for my every day work environment so I will see the problems much sooner. Did we ever get in touch with the Coverity folks about enabling Coverity Scan[1] for libvirt? QEMU is already part of the program. No idea, but we can register there ourselves, can't we? They have integration with GitHub where we have the read-only copy anyway, we can even run in on TravisCI and have the whole bundle without changing much internally (just committing settings file into the repository). I think we would get 2 builds per day, depending on how the github repo is updated. Is that done automatically or every X minutes or something? Anyway it could be nice to have that. [1] https://scan.coverity.com/ -- Andrea Bolognani / Red Hat / Virtualization signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH (RFC?)] Remove usage of __attribute__((nonnull))
On Wed, 2017-04-05 at 06:53 -0400, John Ferlan wrote: > > I thought that John still had a nightly coverity job running that > > would trigger the -DSTATIC_ANALYSIS codepaths. If that's not the case, then > > we'd wnat to look > > at enabling that in one of the centos CI jobs. > > I still have a run of Coverity every night although I have been less > diligent about checking errors lately. Mainly because generally things > that are considered a false positive were being rejected when I posted > patches. I keep about 20 patches in a local branch. > > There was a point quite a few months ago where my nightly build started > failing because either I changed the Coverity version or the compiler > version on my laptop - cannot recall exactly. That resulted in a bunch > more local patches until I finally had too many and posted that pile > late last month. Enabling static analysis in CI builds was something I > suggested in my cover - although now I've done it for my every day work > environment so I will see the problems much sooner. Did we ever get in touch with the Coverity folks about enabling Coverity Scan[1] for libvirt? QEMU is already part of the program. [1] https://scan.coverity.com/ -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 3/8] util: Generate a common internal only error print
If virObjectIsClass fails "internally" to virobject.c, create a macro to
generate the VIR_WARN describing what the problem is. Also improve the
checks and message a bit to indicate which was the failure - whether the
obj was NULL or just not the right class
Signed-off-by: John Ferlan
---
src/util/virobject.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 5d5b6a3..9fe4e71 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -47,6 +47,16 @@ struct _virClass {
virObjectDisposeCallback dispose;
};
+#define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)\
+do {\
+virObjectPtr obj = anyobj; \
+if (!obj) \
+VIR_WARN("Object %p is not a virObject class instance", anyobj);\
+else\
+VIR_WARN("Object %p (%s) is not a %s instance", \
+ anyobj, obj->klass->name, #objclass); \
+} while (0)
+
static virClassPtr virObjectClass;
static virClassPtr virObjectLockableClass;
@@ -312,14 +322,10 @@ 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)");
+VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
return NULL;
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] interface: Introduce virInterfaceObjGetNames
Unlike other drivers, this is a test driver only API. Still combining
the logic of testConnectListInterfaces and testConnectListDefinedInterfaces
makes things a bit easier in the long run.
Signed-off-by: John Ferlan
---
src/conf/virinterfaceobj.c | 34 +
src/conf/virinterfaceobj.h | 6 ++
src/libvirt_private.syms | 1 +
src/test/test_driver.c | 54 +++---
4 files changed, 53 insertions(+), 42 deletions(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 0407c1f..229226a 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -235,3 +235,37 @@ virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr
interfaces,
return ninterfaces;
}
+
+
+int
+virInterfaceObjGetNames(virInterfaceObjListPtr interfaces,
+bool wantActive,
+char **const names,
+int maxnames)
+{
+int nnames = 0;
+size_t i;
+
+for (i = 0; i < interfaces->count && nnames < maxnames; i++) {
+virInterfaceObjPtr obj = interfaces->objs[i];
+virInterfaceObjLock(obj);
+if ((wantActive && virInterfaceObjIsActive(obj)) ||
+(!wantActive && !virInterfaceObjIsActive(obj))) {
+if (VIR_STRDUP(names[nnames], obj->def->name) < 0) {
+virInterfaceObjUnlock(obj);
+goto failure;
+}
+nnames++;
+}
+virInterfaceObjUnlock(obj);
+}
+
+return nnames;
+
+ failure:
+while (--nnames >= 0)
+VIR_FREE(names[nnames]);
+memset(names, 0, maxnames * sizeof(*names));
+
+return -1;
+}
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 2f07174..5b0527d 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -85,4 +85,10 @@ int
virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr interfaces,
bool wantActive);
+int
+virInterfaceObjGetNames(virInterfaceObjListPtr interfaces,
+bool wantActive,
+char **const names,
+int maxnames);
+
#endif /* __VIRINTERFACEOBJ_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 96aacaa..88e530c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -935,6 +935,7 @@ virDomainObjListRename;
virInterfaceObjAssignDef;
virInterfaceObjFindByMACString;
virInterfaceObjFindByName;
+virInterfaceObjGetNames;
virInterfaceObjListClone;
virInterfaceObjListFree;
virInterfaceObjLock;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6910681..4e10eb2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3657,33 +3657,18 @@ static int testConnectNumOfInterfaces(virConnectPtr
conn)
return ninterfaces;
}
-static int testConnectListInterfaces(virConnectPtr conn, char **const names,
int nnames)
+static int testConnectListInterfaces(virConnectPtr conn, char **const names,
int maxnames)
{
testDriverPtr privconn = conn->privateData;
-int n = 0;
-size_t i;
+int nnames;
+
+memset(names, 0, maxnames * sizeof(*names));
testDriverLock(privconn);
-memset(names, 0, sizeof(*names)*nnames);
-for (i = 0; (i < privconn->ifaces.count) && (n < nnames); i++) {
-virInterfaceObjLock(privconn->ifaces.objs[i]);
-if (virInterfaceObjIsActive(privconn->ifaces.objs[i])) {
-if (VIR_STRDUP(names[n++], privconn->ifaces.objs[i]->def->name) <
0) {
-virInterfaceObjUnlock(privconn->ifaces.objs[i]);
-goto error;
-}
-}
-virInterfaceObjUnlock(privconn->ifaces.objs[i]);
-}
+nnames = virInterfaceObjGetNames(&privconn->ifaces, true, names, maxnames);
testDriverUnlock(privconn);
-return n;
-
- error:
-for (n = 0; n < nnames; n++)
-VIR_FREE(names[n]);
-testDriverUnlock(privconn);
-return -1;
+return nnames;
}
static int testConnectNumOfDefinedInterfaces(virConnectPtr conn)
@@ -3697,33 +3682,18 @@ static int
testConnectNumOfDefinedInterfaces(virConnectPtr conn)
return ninterfaces;
}
-static int testConnectListDefinedInterfaces(virConnectPtr conn, char **const
names, int nnames)
+static int testConnectListDefinedInterfaces(virConnectPtr conn, char **const
names, int maxnames)
{
testDriverPtr privconn = conn->privateData;
-int n = 0;
-size_t i;
+int nnames;
+
+memset(names, 0, maxnames * sizeof(*names));
testDriverLock(privconn);
-memset(names, 0, sizeof(*names)*nnames);
-for (i = 0; (i < privconn->ifaces.count) && (n < nnames); i++) {
-virInterfaceObjLock(privconn->ifaces.objs[i]);
-if (!virInterfaceObjIsActive(privconn->ifaces.objs[i])) {
-if (VIR_STRDUP(names[n++], privconn->ifaces.objs[i]->def->name) <
0) {
-virInterfaceObjUnlock(privconn->ifaces.objs[i])
[libvirt] [PATCH 3/3] interface: Clean up Interface section of test_driver
Clean up the code to adhere to more of the standard two spaces between
functions, separate lines for type and function name, one argument per line.
Signed-off-by: John Ferlan
---
src/test/test_driver.c | 81 +++---
1 file changed, 57 insertions(+), 24 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4e10eb2..89c7389 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3646,7 +3646,8 @@ testInterfaceObjFindByName(testDriverPtr privconn,
}
-static int testConnectNumOfInterfaces(virConnectPtr conn)
+static int
+testConnectNumOfInterfaces(virConnectPtr conn)
{
testDriverPtr privconn = conn->privateData;
int ninterfaces;
@@ -3657,7 +3658,11 @@ static int testConnectNumOfInterfaces(virConnectPtr conn)
return ninterfaces;
}
-static int testConnectListInterfaces(virConnectPtr conn, char **const names,
int maxnames)
+
+static int
+testConnectListInterfaces(virConnectPtr conn,
+ char **const names,
+ int maxnames)
{
testDriverPtr privconn = conn->privateData;
int nnames;
@@ -3671,7 +3676,9 @@ static int testConnectListInterfaces(virConnectPtr conn,
char **const names, int
return nnames;
}
-static int testConnectNumOfDefinedInterfaces(virConnectPtr conn)
+
+static int
+testConnectNumOfDefinedInterfaces(virConnectPtr conn)
{
testDriverPtr privconn = conn->privateData;
int ninterfaces;
@@ -3682,7 +3689,11 @@ static int
testConnectNumOfDefinedInterfaces(virConnectPtr conn)
return ninterfaces;
}
-static int testConnectListDefinedInterfaces(virConnectPtr conn, char **const
names, int maxnames)
+
+static int
+testConnectListDefinedInterfaces(virConnectPtr conn,
+ char **const names,
+ int maxnames)
{
testDriverPtr privconn = conn->privateData;
int nnames;
@@ -3696,8 +3707,10 @@ static int
testConnectListDefinedInterfaces(virConnectPtr conn, char **const nam
return nnames;
}
-static virInterfacePtr testInterfaceLookupByName(virConnectPtr conn,
- const char *name)
+
+static virInterfacePtr
+testInterfaceLookupByName(virConnectPtr conn,
+ const char *name)
{
testDriverPtr privconn = conn->privateData;
virInterfaceObjPtr iface;
@@ -3714,8 +3727,10 @@ static virInterfacePtr
testInterfaceLookupByName(virConnectPtr conn,
return ret;
}
-static virInterfacePtr testInterfaceLookupByMACString(virConnectPtr conn,
- const char *mac)
+
+static virInterfacePtr
+testInterfaceLookupByMACString(virConnectPtr conn,
+ const char *mac)
{
testDriverPtr privconn = conn->privateData;
virInterfaceObjPtr iface;
@@ -3744,7 +3759,9 @@ static virInterfacePtr
testInterfaceLookupByMACString(virConnectPtr conn,
return ret;
}
-static int testInterfaceIsActive(virInterfacePtr iface)
+
+static int
+testInterfaceIsActive(virInterfacePtr iface)
{
testDriverPtr privconn = iface->conn->privateData;
virInterfaceObjPtr obj;
@@ -3761,8 +3778,10 @@ static int testInterfaceIsActive(virInterfacePtr iface)
return ret;
}
-static int testInterfaceChangeBegin(virConnectPtr conn,
-unsigned int flags)
+
+static int
+testInterfaceChangeBegin(virConnectPtr conn,
+ unsigned int flags)
{
testDriverPtr privconn = conn->privateData;
int ret = -1;
@@ -3788,8 +3807,10 @@ static int testInterfaceChangeBegin(virConnectPtr conn,
return ret;
}
-static int testInterfaceChangeCommit(virConnectPtr conn,
- unsigned int flags)
+
+static int
+testInterfaceChangeCommit(virConnectPtr conn,
+ unsigned int flags)
{
testDriverPtr privconn = conn->privateData;
int ret = -1;
@@ -3816,8 +3837,10 @@ static int testInterfaceChangeCommit(virConnectPtr conn,
return ret;
}
-static int testInterfaceChangeRollback(virConnectPtr conn,
- unsigned int flags)
+
+static int
+testInterfaceChangeRollback(virConnectPtr conn,
+unsigned int flags)
{
testDriverPtr privconn = conn->privateData;
int ret = -1;
@@ -3848,8 +3871,10 @@ static int testInterfaceChangeRollback(virConnectPtr
conn,
return ret;
}
-static char *testInterfaceGetXMLDesc(virInterfacePtr iface,
- unsigned int flags)
+
+static char *
+testInterfaceGetXMLDesc(virInterfacePtr iface,
+unsigned int flags)
{
testDriverPtr privconn = iface->conn->privateData;
virInterfaceObjPtr privinterface;
@@ -3869,8 +3894,10 @@ static char *testInterfaceGetXMLDesc(virInterfacePtr
iface,
}
-static virInterfacePtr testInterfaceDefineXML(virConnectPtr conn
[libvirt] [PATCH 0/3] interface: Create common virInterface* APIs
Effectively move code from test_driver.c into virinterfaceobj to count the number of devices and to return a list of names. Patch 3 is a bit out of order from my norm, but at least I didn't forget... FWIW: This is part of the common driver objects code I've been working through. I figured I will post each driver separately rather than one larger series. John Ferlan (3): interface: Introduce virInterfaceObjNumOfInterfaces interface: Introduce virInterfaceObjGetNames interface: Clean up Interface section of test_driver src/conf/virinterfaceobj.c | 54 src/conf/virinterfaceobj.h | 10 +++ src/libvirt_private.syms | 2 + src/test/test_driver.c | 155 + 4 files changed, 139 insertions(+), 82 deletions(-) -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] interface: Introduce virInterfaceObjNumOfInterfaces
Unlike other drivers, this is a test driver only API. Still combining
the logic of testConnectNumOfInterfaces and testConnectNumOfDefinedInterfaces
makes things a bit easier in the long run.
Signed-off-by: John Ferlan
---
src/conf/virinterfaceobj.c | 20
src/conf/virinterfaceobj.h | 4
src/libvirt_private.syms | 1 +
src/test/test_driver.c | 24 ++--
4 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 3af972b..0407c1f 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -215,3 +215,23 @@ virInterfaceObjRemove(virInterfaceObjListPtr interfaces,
virInterfaceObjUnlock(interfaces->objs[i]);
}
}
+
+
+int
+virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr interfaces,
+ bool wantActive)
+{
+size_t i;
+int ninterfaces = 0;
+
+for (i = 0; (i < interfaces->count); i++) {
+virInterfaceObjPtr obj = interfaces->objs[i];
+virInterfaceObjLock(obj);
+if ((wantActive && virInterfaceObjIsActive(obj)) ||
+(!wantActive && !virInterfaceObjIsActive(obj)))
+ninterfaces++;
+virInterfaceObjUnlock(obj);
+}
+
+return ninterfaces;
+}
diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
index 6c5e2e7..2f07174 100644
--- a/src/conf/virinterfaceobj.h
+++ b/src/conf/virinterfaceobj.h
@@ -81,4 +81,8 @@ typedef bool
(*virInterfaceObjListFilter)(virConnectPtr conn,
virInterfaceDefPtr def);
+int
+virInterfaceObjNumOfInterfaces(virInterfaceObjListPtr interfaces,
+ bool wantActive);
+
#endif /* __VIRINTERFACEOBJ_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92083e5..96aacaa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -938,6 +938,7 @@ virInterfaceObjFindByName;
virInterfaceObjListClone;
virInterfaceObjListFree;
virInterfaceObjLock;
+virInterfaceObjNumOfInterfaces;
virInterfaceObjRemove;
virInterfaceObjUnlock;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cce4d2d..6910681 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3649,18 +3649,12 @@ testInterfaceObjFindByName(testDriverPtr privconn,
static int testConnectNumOfInterfaces(virConnectPtr conn)
{
testDriverPtr privconn = conn->privateData;
-size_t i;
-int count = 0;
+int ninterfaces;
testDriverLock(privconn);
-for (i = 0; (i < privconn->ifaces.count); i++) {
-virInterfaceObjLock(privconn->ifaces.objs[i]);
-if (virInterfaceObjIsActive(privconn->ifaces.objs[i]))
-count++;
-virInterfaceObjUnlock(privconn->ifaces.objs[i]);
-}
+ninterfaces = virInterfaceObjNumOfInterfaces(&privconn->ifaces, true);
testDriverUnlock(privconn);
-return count;
+return ninterfaces;
}
static int testConnectListInterfaces(virConnectPtr conn, char **const names,
int nnames)
@@ -3695,18 +3689,12 @@ static int testConnectListInterfaces(virConnectPtr
conn, char **const names, int
static int testConnectNumOfDefinedInterfaces(virConnectPtr conn)
{
testDriverPtr privconn = conn->privateData;
-size_t i;
-int count = 0;
+int ninterfaces;
testDriverLock(privconn);
-for (i = 0; i < privconn->ifaces.count; i++) {
-virInterfaceObjLock(privconn->ifaces.objs[i]);
-if (!virInterfaceObjIsActive(privconn->ifaces.objs[i]))
-count++;
-virInterfaceObjUnlock(privconn->ifaces.objs[i]);
-}
+ninterfaces = virInterfaceObjNumOfInterfaces(&privconn->ifaces, false);
testDriverUnlock(privconn);
-return count;
+return ninterfaces;
}
static int testConnectListDefinedInterfaces(virConnectPtr conn, char **const
names, int nnames)
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 01:25:35PM +0100, Daniel P. Berrange wrote: On Thu, Apr 06, 2017 at 08:20:56PM +0800, Eli Qiao wrote: This patch is based on Martin's cache branch. This patch amends the cache bank capability as follow: Why do we need to report 'type' on both bank & control elements. Are they really expected to have different values ? That's one of my questions I had way back in some of the previous discussions. Did not get the answer. I suspect there is a reason to whether CDP is enabled or not and it is not just the type of the cache bank itself. If it is, then CDP makes no sense for us at all, actually for anyone who has access to cache information and that would mean bad design of that thing. And that's not something I would expect from this functionality. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 02:46:10PM +0200, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 08:20:56PM +0800, Eli Qiao wrote: > > This patch is based on Martin's cache branch. > > > > This patch amends the cache bank capability as follow: > > > > It helps a lot if you wait for a conclusion on a patch before sending > another version as soon as you can change one line. > > > > > > > > > > > > > > > > > > > > > I know Dan proposed "nallocations", but it sounds like one word. I > would rather use "allocations" or "max_allocs" or something > understandable. The reason for it? We have no documentation for our > capabilities XML. And nobody is trying to create one as far as I know. > So at least the naming should be more intuitive. I won't mind either of these alternatives. BTW, we really ought to fix the documentation gap too :-) I'm surprised we have gone so long without documenting this key area of functionality! Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 08:20:56PM +0800, Eli Qiao wrote:
This patch is based on Martin's cache branch.
This patch amends the cache bank capability as follow:
It helps a lot if you wait for a conclusion on a patch before sending
another version as soon as you can change one line.
I know Dan proposed "nallocations", but it sounds like one word. I
would rather use "allocations" or "max_allocs" or something
understandable. The reason for it? We have no documentation for our
capabilities XML. And nobody is trying to create one as far as I know.
So at least the naming should be more intuitive.
Along with vircaps2xmltest case updated.
I'm sensing you haven't ran the tests. Am I right?
Signed-off-by: Eli Qiao
---
src/conf/capabilities.c | 120 ++-
src/conf/capabilities.h | 13 ++-
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
tests/vircaps2xmltest.c | 13 +++
4 files changed, 148 insertions(+), 6 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 416dd1a..3094e48 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -52,6 +52,7 @@
#define VIR_FROM_THIS VIR_FROM_CAPABILITIES
#define SYSFS_SYSTEM_PATH "/sys/devices/system/"
+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
VIR_LOG_INIT("conf.capabilities")
@@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
virCapsHostCacheBankPtr *caches)
{
size_t i = 0;
+size_t j = 0;
if (!ncaches)
return 0;
@@ -894,13 +896,32 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
*/
virBufferAsprintf(buf,
"\n",
+ "size='%llu' unit='%s' cpus='%s'",
bank->id, bank->level,
virCacheTypeToString(bank->type),
bank->size >> (kilos * 10),
kilos ? "KiB" : "B",
cpus_str);
+if (bank->ncontrol > 0) {
+virBufferAddLit(buf, ">\n");
+virBufferAdjustIndent(buf, 2);
+for (j = 0; j < bank->ncontrol; j++) {
+bool min_kilos = !(bank->controls[j]->min % 1024);
+virBufferAsprintf(buf,
+ "\n",
+ bank->controls[j]->min >> (min_kilos * 10),
+ min_kilos ? "KiB" : "B",
+
virCacheTypeToString(bank->controls[j]->type),
+ bank->controls[j]->nallocations);
+}
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "/>\n");
+}
+
There's virBufferAddBuffer() for easier usage of this. Just grep for
childrenBuf and you'll see the usage.
VIR_FREE(cpus_str);
}
@@ -1513,13 +1534,97 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
void
virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
{
+size_t i;
+
if (!ptr)
return;
virBitmapFree(ptr->cpus);
+for (i = 0; i < ptr->ncontrol; i++)
+VIR_FREE(ptr->controls[i]);
+VIR_FREE(ptr->controls);
VIR_FREE(ptr);
}
+/* test which kinds of cache control supported
+ * -1: don't support
+ * 0: cat
+ * 1: cdp
+ */
+static int
+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
+{
+int ret = -1;
+char *path = NULL;
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
+return -1;
+
+if (virFileExists(path)) {
+ret = 0;
+} else {
+VIR_FREE(path);
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level)
< 0)
+return -1;
+if (virFileExists(path))
+ret = 1;
+}
+
+VIR_FREE(path);
+return ret;
+}
+
+static int
+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char* type)
+{
+int ret = -1;
+char *path = NULL;
+char *cbm_mask = NULL;
+virCapsHostCacheControlPtr control;
+
+if (VIR_ALLOC(control) < 0)
+goto cleanup;
+
+if (virFileReadValueUint(&control->nallocations,
+ SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+if (virFileReadValueString(&cbm_mask,
+ SYSFS_RESCTRL_PATH
+ "info/L%u%s/cbm_mask",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+virStringTrimOptionalNewline(cbm_mask);
+
+control->min = bank->size / (strlen(cbm_mask) * 4);
+
+if (STREQ("", type))
+control->type = VIR_CACHE_TYPE_UNIFIED;
+else if (STREQ("CODE", type))
+control->type = VIR_CACHE_T
Re: [libvirt] [PATCH] qemu: Fix regression when hyperv/vendor_id feature is used
On Thu, Apr 06, 2017 at 14:28:09 +0200, Peter Krempa wrote: > On Thu, Apr 06, 2017 at 14:20:39 +0200, Jiri Denemark wrote: > > qemuProcessVerifyHypervFeatures is supposed to check whether all > > requested hyperv features were actually honored by QEMU/KVM. This is > > done by checking the corresponding CPUID bits reported by the virtual > > CPU. In other words, it doesn't work for string properties, such as > > VIR_DOMAIN_HYPERV_VENDOR_ID (there is no CPUID bit we could check). We > > could theoretically check all 12 bits corresponding to the vendor > > 12 bytes Oops, I used 96 bits as "bytes" sounds strange to me in CPUID context. > ACK Thanks, pushed to both master and v3.2-maint. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
2017-04-06 15:18 GMT+03:00 Daniel P. Berrange : > Sure you're free to check if you don't believe me. I'm just making suggestion > to save you wasting your time. Ok =) -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix regression when hyperv/vendor_id feature is used
On Thu, Apr 06, 2017 at 14:20:39 +0200, Jiri Denemark wrote: > qemuProcessVerifyHypervFeatures is supposed to check whether all > requested hyperv features were actually honored by QEMU/KVM. This is > done by checking the corresponding CPUID bits reported by the virtual > CPU. In other words, it doesn't work for string properties, such as > VIR_DOMAIN_HYPERV_VENDOR_ID (there is no CPUID bit we could check). We > could theoretically check all 12 bits corresponding to the vendor 12 bytes > string, but luckily we don't have to check the feature at all. If QEMU > is too old to support hyperv features, the domain won't even start. > Otherwise, it is always supported. > > Without this patch, libvirt refuses to start a domain which contains > > > > > > > > reporting internal error: "unknown CPU feature __kvm_hv_vendor_id. > > This regression was introduced by commit v3.1.0-186-ge9dbe7011, which > (by fixing the virCPUDataCheckFeature condition in > qemuProcessVerifyHypervFeatures) revealed an old bug in the feature > verification code. It's been there ever since the verification was > implemented by commit v1.3.3-rc1-5-g95bbe4bf5, which effectively did not > check VIR_DOMAIN_HYPERV_VENDOR_ID at all. > > https://bugzilla.redhat.com/show_bug.cgi?id=1439424 > > Signed-off-by: Jiri Denemark > --- > > BTW, I think this should be cherry-picked in v3.2-maint branch. Yes > > src/qemu/qemu_process.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/12] util: Add virStringTrimOptionalNewline
On Thu, Apr 06, 2017 at 01:32:13PM +0200, Erik Skultety wrote:
On Wed, Apr 05, 2017 at 04:36:26PM +0200, Martin Kletzander wrote:
And use it in virFileRead*
Signed-off-by: Martin Kletzander
---
src/util/virfile.c| 18 +++---
src/util/virhostcpu.c | 4 ++--
src/util/virstring.h | 8
src/util/virsysfs.c | 2 ++
4 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index c0f448d3437d..cbfa3849d793 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3811,7 +3811,6 @@ int
virFileReadValueInt(const char *path, int *value)
{
char *str = NULL;
-char *endp = NULL;
if (!virFileExists(path))
return -2;
@@ -3819,8 +3818,9 @@ virFileReadValueInt(const char *path, int *value)
if (virFileReadAll(path, INT_STRLEN_BOUND(*value), &str) < 0)
return -1;
-if (virStrToLong_i(str, &endp, 10, value) < 0 ||
-(endp && !c_isspace(*endp))) {
+virStringTrimOptionalNewline(str);
+
+if (virStrToLong_i(str, NULL, 10, value) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid integer value '%s' in file '%s'"),
str, path);
@@ -3847,7 +3847,6 @@ int
virFileReadValueUint(const char *path, unsigned int *value)
{
char *str = NULL;
-char *endp = NULL;
if (!virFileExists(path))
return -2;
@@ -3855,8 +3854,9 @@ virFileReadValueUint(const char *path, unsigned int
*value)
if (virFileReadAll(path, INT_STRLEN_BOUND(*value), &str) < 0)
return -1;
-if (virStrToLong_uip(str, &endp, 10, value) < 0 ||
-(endp && !c_isspace(*endp))) {
+virStringTrimOptionalNewline(str);
+
+if (virStrToLong_uip(str, NULL, 10, value)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid unsigned integer value '%s' in file '%s'"),
str, path);
@@ -3886,7 +3886,6 @@ virFileReadValueBitmap(const char *path,
{
char *buf = NULL;
int ret = -1;
-char *tmp = NULL;
if (!virFileExists(path))
return -2;
@@ -3894,10 +3893,7 @@ virFileReadValueBitmap(const char *path,
if (virFileReadAll(path, maxlen, &buf) < 0)
goto cleanup;
-/* trim optinoal newline at the end */
-tmp = buf + strlen(buf) - 1;
-if (*tmp == '\n')
-*tmp = '\0';
+virStringTrimOptionalNewline(buf);
*value = virBitmapParseUnlimited(buf);
if (!*value)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 02b9fc8eb94f..a660e3f4dbe5 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -847,13 +847,13 @@ virHostCPUParseCountLinux(void)
tmp = str;
do {
if (virStrToLong_i(tmp, &tmp, 10, &ret) < 0 ||
-!strchr(",-\n", *tmp)) {
+!strchr(",-", *tmp)) {
virReportError(VIR_ERR_NO_SUPPORT,
_("failed to parse %s"), str);
ret = -1;
goto cleanup;
}
-} while (*tmp++ != '\n');
+} while (*tmp++ && *tmp);
ret++;
cleanup:
diff --git a/src/util/virstring.h b/src/util/virstring.h
index a5550e30d2e2..603650aa1588 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -288,4 +288,12 @@ bool virStringBufferIsPrintable(const uint8_t *buf, size_t
buflen);
char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
+static inline void
+virStringTrimOptionalNewline(char *str)
+{
+char *tmp = str + strlen(str) - 1;
+if (*tmp == '\n')
+*tmp = '\0';
+}
Is there any other reason for using this instead of virTrimSpaces than just
being a bit faster? Because I think the performance gain in this case compared
to 1 iteration of the while loop is very small, thus if possible I would avoid
creating a function for it when there is virTrimSpaces (and I think
virSkipSpacesBackwards would be usable too).
So, several factors:
1) I wasn't looking for functions that would do what I do here, I just
didn't want to be open-coding these three lines all the time.
2) I didn't want it to be a separate function (hence static inline in
the header file).
3) I didn't know we have functions like this.
4) Looking at these function I really don't like them. It's the
precise example on how trying to do everything makes it more
useless. It's doing super easy tiny thing that you want, but
because it "configurable", the code is..., well, let's say "not
very nice".
Having said that, I'm perfectly fine with changing it to using
virTrimSpaces() and hating those functions in my own free time. I'll
just add them to my list.
Other than that, it makes perfect sense to me, ACK.
Erik
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 08:20:56PM +0800, Eli Qiao wrote: > This patch is based on Martin's cache branch. > > This patch amends the cache bank capability as follow: > > > > > Why do we need to report 'type' on both bank & control elements. Are they really expected to have different values ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix regression when hyperv/vendor_id feature is used
qemuProcessVerifyHypervFeatures is supposed to check whether all
requested hyperv features were actually honored by QEMU/KVM. This is
done by checking the corresponding CPUID bits reported by the virtual
CPU. In other words, it doesn't work for string properties, such as
VIR_DOMAIN_HYPERV_VENDOR_ID (there is no CPUID bit we could check). We
could theoretically check all 12 bits corresponding to the vendor
string, but luckily we don't have to check the feature at all. If QEMU
is too old to support hyperv features, the domain won't even start.
Otherwise, it is always supported.
Without this patch, libvirt refuses to start a domain which contains
reporting internal error: "unknown CPU feature __kvm_hv_vendor_id.
This regression was introduced by commit v3.1.0-186-ge9dbe7011, which
(by fixing the virCPUDataCheckFeature condition in
qemuProcessVerifyHypervFeatures) revealed an old bug in the feature
verification code. It's been there ever since the verification was
implemented by commit v1.3.3-rc1-5-g95bbe4bf5, which effectively did not
check VIR_DOMAIN_HYPERV_VENDOR_ID at all.
https://bugzilla.redhat.com/show_bug.cgi?id=1439424
Signed-off-by: Jiri Denemark
---
BTW, I think this should be cherry-picked in v3.2-maint branch.
src/qemu/qemu_process.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 18ff1e143..0f64200da 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3793,6 +3793,10 @@ qemuProcessVerifyHypervFeatures(virDomainDefPtr def,
int rc;
for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
+/* always supported string property */
+if (i == VIR_DOMAIN_HYPERV_VENDOR_ID)
+continue;
+
if (def->hyperv_features[i] != VIR_TRISTATE_SWITCH_ON)
continue;
@@ -3821,13 +3825,13 @@ qemuProcessVerifyHypervFeatures(virDomainDefPtr def,
case VIR_DOMAIN_HYPERV_SYNIC:
case VIR_DOMAIN_HYPERV_STIMER:
case VIR_DOMAIN_HYPERV_RESET:
-case VIR_DOMAIN_HYPERV_VENDOR_ID:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("host doesn't support hyperv '%s' feature"),
virDomainHypervTypeToString(i));
return -1;
/* coverity[dead_error_begin] */
+case VIR_DOMAIN_HYPERV_VENDOR_ID:
case VIR_DOMAIN_HYPERV_LAST:
break;
}
--
2.12.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V3] Expose resource control capabilites on cache bank
This patch is based on Martin's cache branch.
This patch amends the cache bank capability as follow:
Along with vircaps2xmltest case updated.
Signed-off-by: Eli Qiao
---
src/conf/capabilities.c | 120 ++-
src/conf/capabilities.h | 13 ++-
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 8 +-
tests/vircaps2xmltest.c | 13 +++
4 files changed, 148 insertions(+), 6 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 416dd1a..3094e48 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -52,6 +52,7 @@
#define VIR_FROM_THIS VIR_FROM_CAPABILITIES
#define SYSFS_SYSTEM_PATH "/sys/devices/system/"
+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
VIR_LOG_INIT("conf.capabilities")
@@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
virCapsHostCacheBankPtr *caches)
{
size_t i = 0;
+size_t j = 0;
if (!ncaches)
return 0;
@@ -894,13 +896,32 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
*/
virBufferAsprintf(buf,
"\n",
+ "size='%llu' unit='%s' cpus='%s'",
bank->id, bank->level,
virCacheTypeToString(bank->type),
bank->size >> (kilos * 10),
kilos ? "KiB" : "B",
cpus_str);
+if (bank->ncontrol > 0) {
+virBufferAddLit(buf, ">\n");
+virBufferAdjustIndent(buf, 2);
+for (j = 0; j < bank->ncontrol; j++) {
+bool min_kilos = !(bank->controls[j]->min % 1024);
+virBufferAsprintf(buf,
+ "\n",
+ bank->controls[j]->min >> (min_kilos * 10),
+ min_kilos ? "KiB" : "B",
+
virCacheTypeToString(bank->controls[j]->type),
+ bank->controls[j]->nallocations);
+}
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+} else {
+virBufferAddLit(buf, "/>\n");
+}
+
VIR_FREE(cpus_str);
}
@@ -1513,13 +1534,97 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
void
virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
{
+size_t i;
+
if (!ptr)
return;
virBitmapFree(ptr->cpus);
+for (i = 0; i < ptr->ncontrol; i++)
+VIR_FREE(ptr->controls[i]);
+VIR_FREE(ptr->controls);
VIR_FREE(ptr);
}
+/* test which kinds of cache control supported
+ * -1: don't support
+ * 0: cat
+ * 1: cdp
+ */
+static int
+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
+{
+int ret = -1;
+char *path = NULL;
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
+return -1;
+
+if (virFileExists(path)) {
+ret = 0;
+} else {
+VIR_FREE(path);
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level)
< 0)
+return -1;
+if (virFileExists(path))
+ret = 1;
+}
+
+VIR_FREE(path);
+return ret;
+}
+
+static int
+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char* type)
+{
+int ret = -1;
+char *path = NULL;
+char *cbm_mask = NULL;
+virCapsHostCacheControlPtr control;
+
+if (VIR_ALLOC(control) < 0)
+goto cleanup;
+
+if (virFileReadValueUint(&control->nallocations,
+ SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+if (virFileReadValueString(&cbm_mask,
+ SYSFS_RESCTRL_PATH
+ "info/L%u%s/cbm_mask",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+virStringTrimOptionalNewline(cbm_mask);
+
+control->min = bank->size / (strlen(cbm_mask) * 4);
+
+if (STREQ("", type))
+control->type = VIR_CACHE_TYPE_UNIFIED;
+else if (STREQ("CODE", type))
+control->type = VIR_CACHE_TYPE_INSTRUCTION;
+else if (STREQ("DATA", type))
+control->type = VIR_CACHE_TYPE_DATA;
+else
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(bank->controls,
+ bank->ncontrol,
+ control) < 0)
+goto error;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+return ret;
+ error:
+VIR_FREE(control);
+return -1;
+}
+
int
virCapabilitiesInitCaches(virCapsPtr caps)
{
@@ -1595,7 +1700,18 @@ virCapabilitiesInitCaches(virCapsPtr caps)
pos, ent->d_name) < 0)
goto cleanup;
-
Re: [libvirt] creating stream
On Thu, Apr 06, 2017 at 03:16:44PM +0300, Vasiliy Tolstov wrote: > 2017-04-06 15:09 GMT+03:00 Daniel P. Berrange : > > Hiding multiple hosts behind the libvirt API doesn't really work for the > > majority of our API objects, as the object model is designed around a > > single host concept. You need a different level of API abstraction to > > effectively manage a pool of multiple hosts. > > > May be you right. But why not try to do and check... Sure you're free to check if you don't believe me. I'm just making suggestion to save you wasting your time. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
2017-04-06 15:09 GMT+03:00 Daniel P. Berrange : > Hiding multiple hosts behind the libvirt API doesn't really work for the > majority of our API objects, as the object model is designed around a > single host concept. You need a different level of API abstraction to > effectively manage a pool of multiple hosts. May be you right. But why not try to do and check... -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
On Thu, Apr 06, 2017 at 03:09:12PM +0300, Vasiliy Tolstov wrote: > 2017-04-06 15:06 GMT+03:00 Vasiliy Tolstov : > >> We already have a fine grained access control system that can be used to > >> restrict feature access... > > > Also i don't think that libvirt access control have ability to deny > access based on function and payload. For example i need to deny for > some users ability to create domain with memory more then 10G or > network with type nat. Trying todo that kind of config access control at the libvirt API level is doomed to failure. The ability to pass in an XML document describing a guest gives you privileges equivalent to root. There are so many ways you can use the XML document give a guest access to host resources, that trying to do access restrictions based on XML content is impractical. You are inevitably going to miss the existance of certain XML features and thus think you have a locked down system where in fact you've left plenty of backdoors to exploit it. Every time QEMU or libvirt are upgraded new features are introduced so again what you thought was secure may now suddenly be insecure. This is why we explicitly don't expose this info to the access control framework. You need to have a much higher level representation of the guest configuration data & related resources in order to do practical access control on usage of individual guest config features. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 4/8] util: Add magic_marker for all virObjects
The virObject logic "assumes" that whatever is passed to its API's
would be some sort of virObjectPtr; however, if it is not then some
really bad things can happen.
So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
and virObjectIsClass and the virObject and virObjectLockable class
consumers have been well behaved and code well tested. Soon there will
be more consumers and one such consumer tripped over this during testing
by passing a virHashTablePtr to virObjectIsClass which ends up calling
virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
object causing one of those bad things to happen.
To avoid the future possibility that a non virObject class memory was
passed to some virObject* API, let's add a "magic_marker" to the base
virObject class that contains a value "0xFEEDBEEF", then any place in
the code which would accept or process the base opaque virObjectPtr,
compare the magic_marker against 0xFEEDBEEF to make sure this is an
object allocated by this code.
It is still left up to the caller to handle the failed API calls just
as it would be if it passed a NULL opaque pointer anyobj.
Signed-off-by: John Ferlan
---
src/util/virobject.c | 12
src/util/virobject.h | 1 +
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 9fe4e71..aea8a6d 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -47,10 +47,12 @@ struct _virClass {
virObjectDisposeCallback dispose;
};
+#define VIR_OBJECT_NOTVALID(obj) (!obj || obj->magic_marker != 0xFEEDBEEF)
+
#define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)\
do {\
virObjectPtr obj = anyobj; \
-if (!obj) \
+if (VIR_OBJECT_NOTVALID(obj)) \
VIR_WARN("Object %p is not a virObject class instance", anyobj);\
else\
VIR_WARN("Object %p (%s) is not a %s instance", \
@@ -212,6 +214,7 @@ virObjectNew(virClassPtr klass)
return NULL;
obj->u.s.magic = klass->magic;
+obj->magic_marker = 0xFEEDBEEF;
obj->klass = klass;
virAtomicIntSet(&obj->u.s.refs, 1);
@@ -272,7 +275,7 @@ virObjectUnref(void *anyobj)
{
virObjectPtr obj = anyobj;
-if (!obj)
+if (VIR_OBJECT_NOTVALID(obj))
return false;
bool lastRef = virAtomicIntDecAndTest(&obj->u.s.refs);
@@ -289,6 +292,7 @@ virObjectUnref(void *anyobj)
/* Clear & poison object */
memset(obj, 0, obj->klass->objectSize);
obj->u.s.magic = 0xDEADBEEF;
+obj->magic_marker = 0xDEADBEEF;
obj->klass = (void*)0xDEADBEEF;
VIR_FREE(obj);
}
@@ -311,7 +315,7 @@ virObjectRef(void *anyobj)
{
virObjectPtr obj = anyobj;
-if (!obj)
+if (VIR_OBJECT_NOTVALID(obj))
return NULL;
virAtomicIntInc(&obj->u.s.refs);
PROBE(OBJECT_REF, "obj=%p", obj);
@@ -390,7 +394,7 @@ virObjectIsClass(void *anyobj,
virClassPtr klass)
{
virObjectPtr obj = anyobj;
-if (!obj)
+if (VIR_OBJECT_NOTVALID(obj))
return false;
return virClassIsDerivedFrom(obj->klass, klass);
diff --git a/src/util/virobject.h b/src/util/virobject.h
index f4c292b..89f8050 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -51,6 +51,7 @@ struct _virObject {
int refs;
} s;
} u;
+unsigned int magic_marker;
virClassPtr klass;
};
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 5/8] util: Introduce virObjectPoolableHashTable
Add the initial infrastructure to define a new object which will be
a child of the virObjectLockable class that can be used to create an
object that has two hash tables for object consumers to be able to
utilize to store elements in one or two hash tables depending on the
need to have more than one lookup mechanism for the same object
TODO: Remove VIR_DEBUG's or add separately
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 2 ++
src/util/virobject.c | 72 +++-
src/util/virobject.h | 29 +++
3 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92083e5..6861155 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2223,6 +2223,7 @@ virNumaSetupMemoryPolicy;
# util/virobject.h
virClassForObject;
virClassForObjectLockable;
+virClassForObjectPoolableHashTable;
virClassIsDerivedFrom;
virClassName;
virClassNew;
@@ -2234,6 +2235,7 @@ virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
virObjectNew;
+virObjectPoolableHashTableNew;
virObjectRef;
virObjectUnlock;
virObjectUnref;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index aea8a6d..d284dde 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -61,8 +61,11 @@ struct _virClass {
static virClassPtr virObjectClass;
static virClassPtr virObjectLockableClass;
+static virClassPtr virObjectPoolableHashTableClass;
static void virObjectLockableDispose(void *anyobj);
+static void virObjectPoolableHashTableDispose(void *anyobj);
+
static int
virObjectOnceInit(void)
@@ -79,6 +82,13 @@ virObjectOnceInit(void)
virObjectLockableDispose)))
return -1;
+if (!(virObjectPoolableHashTableClass =
+ virClassNew(virObjectLockableClass,
+ "virObjectPoolableHashTable",
+ sizeof(virObjectPoolableHashTable),
+ virObjectPoolableHashTableDispose)))
+return -1;
+
return 0;
}
@@ -96,6 +106,7 @@ virClassForObject(void)
if (virObjectInitialize() < 0)
return NULL;
+VIR_DEBUG("virObjectClass=%p", virObjectClass);
return virObjectClass;
}
@@ -111,11 +122,29 @@ virClassForObjectLockable(void)
if (virObjectInitialize() < 0)
return NULL;
+VIR_DEBUG("virObjectLockableClass=%p", virObjectLockableClass);
return virObjectLockableClass;
}
/**
+ * virClassForObjectPoolableHashTable:
+ *
+ * Returns the class instance for the virObjectPoolableHashTable type
+ */
+virClassPtr
+virClassForObjectPoolableHashTable(void)
+{
+if (virObjectInitialize() < 0)
+return NULL;
+
+VIR_DEBUG("virObjectPoolableHashTableClass=%p",
+ virObjectPoolableHashTableClass);
+return virObjectPoolableHashTableClass;
+}
+
+
+/**
* virClassNew:
* @parent: the parent class
* @name: the class name
@@ -139,6 +168,9 @@ virClassNew(virClassPtr parent,
{
virClassPtr klass;
+VIR_DEBUG("parent=%p, name=%s, objectSize=%zu, dispose=%p",
+ parent, name, objectSize, dispose);
+
if (parent == NULL &&
STRNEQ(name, "virObject")) {
virReportInvalidNonNullArg(parent);
@@ -246,6 +278,7 @@ virObjectLockableNew(virClassPtr klass)
return NULL;
}
+VIR_DEBUG("lockable new obj=%p", obj);
return obj;
}
@@ -255,10 +288,46 @@ virObjectLockableDispose(void *anyobj)
{
virObjectLockablePtr obj = anyobj;
+VIR_DEBUG("lockable dispose obj=%p", obj);
virMutexDestroy(&obj->lock);
}
+void *
+virObjectPoolableHashTableNew(virClassPtr klass,
+ virObjectPoolableHashTableObjType objtype)
+{
+virObjectPoolableHashTablePtr obj;
+
+if (!virClassIsDerivedFrom(klass, virClassForObjectPoolableHashTable())) {
+virReportInvalidArg(klass,
+_("Class %s must derive from "
+ "virObjectPoolableHashTable"),
+virClassName(klass));
+return NULL;
+}
+
+if (!(obj = virObjectLockableNew(klass)))
+return NULL;
+
+obj->objtype = objtype;
+
+VIR_DEBUG("poolable new obj=%p, type=%d",
+ obj, objtype);
+
+return obj;
+
+}
+
+static void
+virObjectPoolableHashTableDispose(void *anyobj)
+{
+virObjectPoolableHashTablePtr obj = anyobj;
+
+VIR_DEBUG("poolable dispose obj=%p", obj);
+}
+
+
/**
* virObjectUnref:
* @anyobj: any instance of virObjectPtr
@@ -326,7 +395,8 @@ virObjectRef(void *anyobj)
static virObjectLockablePtr
virObjectGetLockableObj(void *anyobj)
{
-if (virObjectIsClass(anyobj, virObjectLockableClass))
+if (virObjectIsClass(anyobj, virObjectLockableClass) ||
+virObjectIsClass(anyobj, virObjectPoolableHashTableClass))
return anyobj;
VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
diff --git a/
[libvirt] [RFC PATCH 6/8] util: Add the hash tables to virObjectPoolableHashTable
Add a pair of hash tables to the object - a primary and a secondary one.
If the consumer requested only a primary, then only it will be created.
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 2 ++
src/util/virobject.c | 78 +---
src/util/virobject.h | 22 +-
3 files changed, 97 insertions(+), 5 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6861155..9b3345a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2235,6 +2235,8 @@ virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
virObjectNew;
+virObjectPoolableHashTableGetPrimary;
+virObjectPoolableHashTableGetSecondary;
virObjectPoolableHashTableNew;
virObjectRef;
virObjectUnlock;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index d284dde..625bf90 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -295,7 +295,9 @@ virObjectLockableDispose(void *anyobj)
void *
virObjectPoolableHashTableNew(virClassPtr klass,
- virObjectPoolableHashTableObjType objtype)
+ virObjectPoolableHashTableObjType objtype,
+ int tableElemsStart,
+ bool primaryOnly)
{
virObjectPoolableHashTablePtr obj;
@@ -311,20 +313,39 @@ virObjectPoolableHashTableNew(virClassPtr klass,
return NULL;
obj->objtype = objtype;
+obj->tableElemsStart = tableElemsStart;
-VIR_DEBUG("poolable new obj=%p, type=%d",
- obj, objtype);
+if (!(obj->objsPrimary = virHashCreate(tableElemsStart,
+ virObjectFreeHashData)))
+goto error;
+
+if (!primaryOnly &&
+!(obj->objsSecondary = virHashCreate(tableElemsStart,
+ virObjectFreeHashData)))
+goto error;
+
+
+VIR_DEBUG("obj=%p, type=%d, elems=%d objsPrimary=%p objsSecondary=%p",
+ obj, objtype, tableElemsStart,
+ obj->objsPrimary, obj->objsSecondary);
return obj;
+ error:
+virObjectUnref(obj);
+return NULL;
}
+
static void
virObjectPoolableHashTableDispose(void *anyobj)
{
virObjectPoolableHashTablePtr obj = anyobj;
VIR_DEBUG("poolable dispose obj=%p", obj);
+
+virHashFree(obj->objsPrimary);
+virHashFree(obj->objsSecondary);
}
@@ -402,7 +423,6 @@ virObjectGetLockableObj(void *anyobj)
VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
return NULL;
-
}
@@ -557,3 +577,53 @@ virObjectListFreeCount(void *list,
VIR_FREE(list);
}
+
+
+static virObjectPoolableHashTablePtr
+virObjectGetPoolableHashTableObj(void *anyobj)
+{
+if (virObjectIsClass(anyobj, virObjectPoolableHashTableClass))
+return anyobj;
+
+VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectPoolableHashTableClass);
+
+return NULL;
+}
+
+
+/**
+ * virObjectPoolableHashTableGetPrimary
+ * @anyobj: Pointer to a PoolableHashTable object
+ *
+ * Returns: Pointer to the Primary Hash Table or NULL on failure
+ */
+virHashTablePtr
+virObjectPoolableHashTableGetPrimary(void *anyobj)
+{
+virObjectPoolableHashTablePtr obj =
+virObjectGetPoolableHashTableObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->objsPrimary;
+}
+
+
+/**
+ * virObjectPoolableHashTableGetSecondary
+ * @anyobj: Pointer to a PoolableHashTable object
+ *
+ * Returns: Pointer to the Secondary Hash Table or NULL on failure
+ */
+virHashTablePtr
+virObjectPoolableHashTableGetSecondary(void *anyobj)
+{
+virObjectPoolableHashTablePtr obj =
+virObjectGetPoolableHashTableObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->objsSecondary;
+}
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 30ce6a1..ac21190 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -81,6 +81,18 @@ typedef enum {
struct _virObjectPoolableHashTable {
virObjectLockable parent;
virObjectPoolableHashTableObjType objtype;
+
+int tableElemsStart;
+
+/* primary key -> object mapping for O(1),
+ * lockless lookup-by-primary */
+virHashTable *objsPrimary;
+
+/* uuid string -> virPoolObj mapping
+ * for O(1), lockless lookup-by-uuid */
+/* secondary key -> object mapping for O(1),
+ * lockless lookup-by-secondary */
+virHashTable *objsSecondary;
};
@@ -135,7 +147,9 @@ virObjectLockableNew(virClassPtr klass)
void *
virObjectPoolableHashTableNew(virClassPtr klass,
- virObjectPoolableHashTableObjType objtype)
+ virObjectPoolableHashTableObjType objtype,
+ int tableElemsStart,
+ bool primaryOnly)
ATTRIBUTE_NONNULL(1);
void
@@ -153,4 +167,10 @@ void
virObjectListFreeCount(void *list,
size_t count);
+virHashTablePtr
+virObjectPoolab
[libvirt] [RFC PATCH 2/8] util: Introduce virObjectGetLockableObj
Split out the object fetch in virObject{Lock|Unlock} into a helper
Signed-off-by: John Ferlan
---
src/util/virobject.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 792685b..5d5b6a3 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -309,6 +309,23 @@ 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)");
+
+return NULL;
+
+}
+
+
/**
* virObjectLock:
* @anyobj: any instance of virObjectLockablePtr
@@ -324,13 +341,10 @@ virObjectRef(void *anyobj)
void
virObjectLock(void *anyobj)
{
-virObjectLockablePtr obj = anyobj;
+virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
-if (!virObjectIsClass(obj, virObjectLockableClass)) {
-VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
- obj, obj ? obj->parent.klass->name : "(unknown)");
+if (!obj)
return;
-}
virMutexLock(&obj->lock);
}
@@ -346,13 +360,10 @@ virObjectLock(void *anyobj)
void
virObjectUnlock(void *anyobj)
{
-virObjectLockablePtr obj = anyobj;
+virObjectLockablePtr obj = virObjectGetLockableObj(anyobj);
-if (!virObjectIsClass(obj, virObjectLockableClass)) {
-VIR_WARN("Object %p (%s) is not a virObjectLockable instance",
- obj, obj ? obj->parent.klass->name : "(unknown)");
+if (!obj)
return;
-}
virMutexUnlock(&obj->lock);
}
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 1/8] util: Formatting cleanups to virobject API
Alter to use more recent formatting guidelines
Signed-off-by: John Ferlan
---
src/util/virobject.c | 64 ++--
src/util/virobject.h | 59
2 files changed, 82 insertions(+), 41 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 51876b9..792685b 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -52,7 +52,8 @@ static virClassPtr virObjectLockableClass;
static void virObjectLockableDispose(void *anyobj);
-static int virObjectOnceInit(void)
+static int
+virObjectOnceInit(void)
{
if (!(virObjectClass = virClassNew(NULL,
"virObject",
@@ -77,7 +78,8 @@ VIR_ONCE_GLOBAL_INIT(virObject);
*
* Returns the class instance for the base virObject type
*/
-virClassPtr virClassForObject(void)
+virClassPtr
+virClassForObject(void)
{
if (virObjectInitialize() < 0)
return NULL;
@@ -91,7 +93,8 @@ virClassPtr virClassForObject(void)
*
* Returns the class instance for the virObjectLockable type
*/
-virClassPtr virClassForObjectLockable(void)
+virClassPtr
+virClassForObjectLockable(void)
{
if (virObjectInitialize() < 0)
return NULL;
@@ -116,10 +119,11 @@ virClassPtr virClassForObjectLockable(void)
*
* Returns a new class instance
*/
-virClassPtr virClassNew(virClassPtr parent,
-const char *name,
-size_t objectSize,
-virObjectDisposeCallback dispose)
+virClassPtr
+virClassNew(virClassPtr parent,
+const char *name,
+size_t objectSize,
+virObjectDisposeCallback dispose)
{
virClassPtr klass;
@@ -162,8 +166,9 @@ virClassPtr virClassNew(virClassPtr parent,
*
* Return true if @klass is derived from @parent, false otherwise
*/
-bool virClassIsDerivedFrom(virClassPtr klass,
- virClassPtr parent)
+bool
+virClassIsDerivedFrom(virClassPtr klass,
+ virClassPtr parent)
{
while (klass) {
if (klass->magic == parent->magic)
@@ -186,7 +191,8 @@ bool virClassIsDerivedFrom(virClassPtr klass,
*
* Returns the new object
*/
-void *virObjectNew(virClassPtr klass)
+void *
+virObjectNew(virClassPtr klass)
{
virObjectPtr obj = NULL;
@@ -205,7 +211,8 @@ void *virObjectNew(virClassPtr klass)
}
-void *virObjectLockableNew(virClassPtr klass)
+void *
+virObjectLockableNew(virClassPtr klass)
{
virObjectLockablePtr obj;
@@ -230,13 +237,15 @@ void *virObjectLockableNew(virClassPtr klass)
}
-static void virObjectLockableDispose(void *anyobj)
+static void
+virObjectLockableDispose(void *anyobj)
{
virObjectLockablePtr obj = anyobj;
virMutexDestroy(&obj->lock);
}
+
/**
* virObjectUnref:
* @anyobj: any instance of virObjectPtr
@@ -248,7 +257,8 @@ static void virObjectLockableDispose(void *anyobj)
* Returns true if the remaining reference count is
* non-zero, false if the object was disposed of
*/
-bool virObjectUnref(void *anyobj)
+bool
+virObjectUnref(void *anyobj)
{
virObjectPtr obj = anyobj;
@@ -286,7 +296,8 @@ bool virObjectUnref(void *anyobj)
*
* Returns @anyobj
*/
-void *virObjectRef(void *anyobj)
+void *
+virObjectRef(void *anyobj)
{
virObjectPtr obj = anyobj;
@@ -310,7 +321,8 @@ void *virObjectRef(void *anyobj)
* The object must be unlocked before releasing this
* reference.
*/
-void virObjectLock(void *anyobj)
+void
+virObjectLock(void *anyobj)
{
virObjectLockablePtr obj = anyobj;
@@ -331,7 +343,8 @@ void virObjectLock(void *anyobj)
* Release a lock on @anyobj. The lock must have been
* acquired by virObjectLock.
*/
-void virObjectUnlock(void *anyobj)
+void
+virObjectUnlock(void *anyobj)
{
virObjectLockablePtr obj = anyobj;
@@ -355,8 +368,9 @@ void virObjectUnlock(void *anyobj)
*
* Returns true if @anyobj is an instance of @klass
*/
-bool virObjectIsClass(void *anyobj,
- virClassPtr klass)
+bool
+virObjectIsClass(void *anyobj,
+ virClassPtr klass)
{
virObjectPtr obj = anyobj;
if (!obj)
@@ -372,7 +386,8 @@ bool virObjectIsClass(void *anyobj,
*
* Returns the name of @klass
*/
-const char *virClassName(virClassPtr klass)
+const char *
+virClassName(virClassPtr klass)
{
return klass->name;
}
@@ -401,7 +416,9 @@ void virObjectFreeCallback(void *opaque)
* but with the signature matching the virHashDataFree
* typedef.
*/
-void virObjectFreeHashData(void *opaque, const void *name ATTRIBUTE_UNUSED)
+void
+virObjectFreeHashData(void *opaque,
+ const void *name ATTRIBUTE_UNUSED)
{
virObjectUnref(opaque);
}
@@ -413,7 +430,8 @@ void virObjectFreeHashData(void *opaque, const void *name
ATTRIBUTE_UNUSED)
*
* Unrefs all members of @list and frees the list itself.
*/
-void virObjectListFree(void *list)
+void
+virObjectListFree(void *list)
{
void
[libvirt] [RFC PATCH 7/8] util: Introduce virObjectPoolableHashElement
Add a new virObjectLockable child which will be used to describe
elements that would be inserted into the virObjectPoolableHashTable.
Each virObjectPoolableHashElement will have a primaryKey and a secondaryKey
which can be used to insert the same object into both of the hash tables
that are part of the virObjectPoolableHashTable. This allows for lookup
of the object by more than one means.
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 2 ++
src/util/virobject.c | 77 ++--
src/util/virobject.h | 17 +++
3 files changed, 94 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9b3345a..74d4bf8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2223,6 +2223,7 @@ virNumaSetupMemoryPolicy;
# util/virobject.h
virClassForObject;
virClassForObjectLockable;
+virClassForObjectPoolableHashElement;
virClassForObjectPoolableHashTable;
virClassIsDerivedFrom;
virClassName;
@@ -2235,6 +2236,7 @@ virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
virObjectNew;
+virObjectPoolableHashElementNew;
virObjectPoolableHashTableGetPrimary;
virObjectPoolableHashTableGetSecondary;
virObjectPoolableHashTableNew;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 625bf90..c9e631c 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -62,10 +62,11 @@ struct _virClass {
static virClassPtr virObjectClass;
static virClassPtr virObjectLockableClass;
static virClassPtr virObjectPoolableHashTableClass;
+static virClassPtr virObjectPoolableHashElementClass;
static void virObjectLockableDispose(void *anyobj);
static void virObjectPoolableHashTableDispose(void *anyobj);
-
+static void virObjectPoolableHashElementDispose(void *anyobj);
static int
virObjectOnceInit(void)
@@ -89,6 +90,13 @@ virObjectOnceInit(void)
virObjectPoolableHashTableDispose)))
return -1;
+if (!(virObjectPoolableHashElementClass =
+ virClassNew(virObjectLockableClass,
+ "virObjectPoolableHashElement",
+ sizeof(virObjectPoolableHashElement),
+ virObjectPoolableHashElementDispose)))
+return -1;
+
return 0;
}
@@ -145,6 +153,23 @@ virClassForObjectPoolableHashTable(void)
/**
+ * virClassForObjectPoolableHashElement:
+ *
+ * Returns the class instance for the virObjectPoolableHashElement type
+ */
+virClassPtr
+virClassForObjectPoolableHashElement(void)
+{
+if (virObjectInitialize() < 0)
+return NULL;
+
+VIR_DEBUG("virObjectPoolableHashElementClass=%p",
+ virObjectPoolableHashElementClass);
+return virObjectPoolableHashElementClass;
+}
+
+
+/**
* virClassNew:
* @parent: the parent class
* @name: the class name
@@ -349,6 +374,53 @@ virObjectPoolableHashTableDispose(void *anyobj)
}
+void *
+virObjectPoolableHashElementNew(virClassPtr klass,
+const char *primaryKey,
+const char *secondaryKey)
+{
+virObjectPoolableHashElementPtr obj;
+
+if (!virClassIsDerivedFrom(klass, virClassForObjectPoolableHashElement()))
{
+virReportInvalidArg(klass,
+_("Class %s must derive from "
+ "virObjectPoolableHashElement"),
+virClassName(klass));
+return NULL;
+}
+
+if (!(obj = virObjectLockableNew(klass)))
+return NULL;
+
+if (VIR_STRDUP(obj->primaryKey, primaryKey) < 0)
+goto error;
+
+if (secondaryKey && VIR_STRDUP(obj->secondaryKey, secondaryKey) < 0)
+goto error;
+
+VIR_DEBUG("obj=%p, primary=%s secondary=%s",
+ obj, obj->primaryKey, NULLSTR(obj->secondaryKey));
+
+return obj;
+
+ error:
+virObjectUnref(obj);
+return NULL;
+
+}
+
+static void
+virObjectPoolableHashElementDispose(void *anyobj)
+{
+virObjectPoolableHashElementPtr obj = anyobj;
+
+VIR_DEBUG("dispose obj=%p", obj);
+
+VIR_FREE(obj->primaryKey);
+VIR_FREE(obj->secondaryKey);
+}
+
+
/**
* virObjectUnref:
* @anyobj: any instance of virObjectPtr
@@ -417,7 +489,8 @@ static virObjectLockablePtr
virObjectGetLockableObj(void *anyobj)
{
if (virObjectIsClass(anyobj, virObjectLockableClass) ||
-virObjectIsClass(anyobj, virObjectPoolableHashTableClass))
+virObjectIsClass(anyobj, virObjectPoolableHashTableClass) ||
+virObjectIsClass(anyobj, virObjectPoolableHashElementClass))
return anyobj;
VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
diff --git a/src/util/virobject.h b/src/util/virobject.h
index ac21190..36fd842 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -38,6 +38,9 @@ typedef virObjectLockable *virObjectLockablePtr;
typedef struct _virObjectPoolableHashTable virObjectPoolableHashTable;
typedef virObjectPoolableHashTable *virObjec
[libvirt] [RFC PATCH 8/8] util: Add virObjectPoolableHashElementGet*Key
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 2 ++
src/util/virobject.c | 51
src/util/virobject.h | 6 ++
3 files changed, 59 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 74d4bf8..b18614b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2236,6 +2236,8 @@ virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
virObjectNew;
+virObjectPoolableHashElementGetPrimaryKey;
+virObjectPoolableHashElementGetSecondaryKey;
virObjectPoolableHashElementNew;
virObjectPoolableHashTableGetPrimary;
virObjectPoolableHashTableGetSecondary;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index c9e631c..87bf56a 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -700,3 +700,54 @@ virObjectPoolableHashTableGetSecondary(void *anyobj)
return obj->objsSecondary;
}
+
+
+static virObjectPoolableHashElementPtr
+virObjectGetPoolableHashElementObj(void *anyobj)
+{
+if (virObjectIsClass(anyobj, virObjectPoolableHashElementClass))
+return anyobj;
+
+VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectPoolableHashElementClass);
+
+return NULL;
+
+}
+
+
+/**
+ * virObjectPoolableHashElementGetPrimaryKey
+ * @anyobj: Pointer to a PoolableHashElement object
+ *
+ * Returns: Pointer to the primaryKey or NULL on failure
+ */
+const char *
+virObjectPoolableHashElementGetPrimaryKey(void *anyobj)
+{
+virObjectPoolableHashElementPtr obj =
+virObjectGetPoolableHashElementObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->primaryKey;
+}
+
+
+/**
+ * virObjectPoolableHashElementGetSecondaryKey
+ * @anyobj: Pointer to a PoolableHashElement object
+ *
+ * Returns: Pointer to the secondaryKey which may be NULL
+ */
+const char *
+virObjectPoolableHashElementGetSecondaryKey(void *anyobj)
+{
+virObjectPoolableHashElementPtr obj =
+virObjectGetPoolableHashElementObj(anyobj);
+
+if (!obj)
+return NULL;
+
+return obj->secondaryKey;
+}
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 36fd842..c1e40da 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -190,4 +190,10 @@ virObjectPoolableHashTableGetPrimary(void *anyobj);
virHashTablePtr
virObjectPoolableHashTableGetSecondary(void *anyobj);
+const char *
+virObjectPoolableHashElementGetPrimaryKey(void *anyobj);
+
+const char *
+virObjectPoolableHashElementGetSecondaryKey(void *anyobj);
+
#endif /* __VIR_OBJECT_H */
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 0/8] util: virObject fixes and enhancements
Posted as RFC mainly for the benefit of patches 5-8. The first 4 patches could be considered non-RFC material, but posting half of each just didn't seem reasonable. Before I get "too far" into this virobject adjustment and inheritance model, I wanted to be sure the chosen names were suitable and that I read the comments from responses to patch 2 of my previous RFC correctly: https://www.redhat.com/archives/libvir-list/2017-February/msg00521.html at least with respect to how the inheritence should work. Secondarily, like a "real object" - should the API's that act upon the object be part of the object? Of course since virObjectLockable didn't do that I figured I wouldn't do so either (eg, rather than virObjectLock and virObjectUnlock, it could have been obj->Lock() and obj->Unlock()). The next 'logical' step after patch 8 would be to create an object that uses virObjectPoolableHashElement that would be able to effectly look like my previous virPoolObj objects insomuch as it'd have the 'def' pointer, all the various flags, etc. That object then would be consumed by each driver rather than virObjectLockable parent. That's where the real fun begins. Once that's there - moving the add, remove, search/find, etc. API's into virobject with callbacks to do vir*obj.c specific things. So far in my local branches I have a working model of the secret driver using everything here. It's been a painful process to extract out patches and move things along "more slowly", but it's allowed at least one optimization that I didn't consider originally - using primary/secondary names rather than assuming UUID/Name. John Ferlan (8): util: Formatting cleanups to virobject API util: Introduce virObjectGetLockableObj util: Generate a common internal only error print util: Add magic_marker for all virObjects util: Introduce virObjectPoolableHashTable util: Add the hash tables to virObjectPoolableHashTable util: Introduce virObjectPoolableHashElement util: Add virObjectPoolableHashElementGet*Key src/libvirt_private.syms | 8 + src/util/virobject.c | 375 ++- src/util/virobject.h | 132 ++--- 3 files changed, 461 insertions(+), 54 deletions(-) -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
On Thu, Apr 06, 2017 at 03:06:49PM +0300, Vasiliy Tolstov wrote: > 2017-04-06 15:05 GMT+03:00 Daniel P. Berrange : > > We already have a fine grained access control system that can be used to > > restrict feature access... > > > I know, but i need to proxy - say have one endpoint for array of > libvirt hosts. Not worry this is fun and i think for someone this is > useful to understand how libvirt works internally. Hiding multiple hosts behind the libvirt API doesn't really work for the majority of our API objects, as the object model is designed around a single host concept. You need a different level of API abstraction to effectively manage a pool of multiple hosts. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
2017-04-06 15:06 GMT+03:00 Vasiliy Tolstov : >> We already have a fine grained access control system that can be used to >> restrict feature access... Also i don't think that libvirt access control have ability to deny access based on function and payload. For example i need to deny for some users ability to create domain with memory more then 10G or network with type nat. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
2017-04-06 15:05 GMT+03:00 Daniel P. Berrange : > We already have a fine grained access control system that can be used to > restrict feature access... I know, but i need to proxy - say have one endpoint for array of libvirt hosts. Not worry this is fun and i think for someone this is useful to understand how libvirt works internally. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 08:09:35PM +0800, Eli Qiao wrote: > > > On Thursday, 6 April 2017 at 7:56 PM, Daniel P. Berrange wrote: > > > On Thu, Apr 06, 2017 at 07:32:59PM +0800, Eli Qiao wrote: > > > This patch is based on Martin's cache branch. > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > > > > cpus='6-11'/> > > > > > > > > > > > > > > > This is still wrong per my previous comments. > > > > > I will repost V3 to change it as your comments. > > one question, if there’s no `control`, what’s the bank should be like? > >
Re: [libvirt] [PATCH V2] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 7:56 PM, Daniel P. Berrange wrote: > On Thu, Apr 06, 2017 at 07:32:59PM +0800, Eli Qiao wrote: > > This patch is based on Martin's cache branch. > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > > > > > > This is still wrong per my previous comments. > > I will repost V3 to change it as your comments. one question, if there’s no `control`, what’s the bank should be like?
Re: [libvirt] creating stream
On Thu, Apr 06, 2017 at 03:01:40PM +0300, Vasiliy Tolstov wrote: > 2017-04-06 14:59 GMT+03:00 Michal Privoznik : > > That is not going to be enough. The RPC protocol is spread among other files > > too. You really should not hook into our RPC protocol. > > > My question is still > > unanswered though - why do you need it anyway? > > Now only for fun, later - i want to try implement libvirt proxy with > ability to selective deny some features in the middle We already have a fine grained access control system that can be used to restrict feature access... Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
2017-04-06 14:59 GMT+03:00 Michal Privoznik : > That is not going to be enough. The RPC protocol is spread among other files > too. You really should not hook into our RPC protocol. > My question is still > unanswered though - why do you need it anyway? Now only for fun, later - i want to try implement libvirt proxy with ability to selective deny some features in the middle -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] creating stream
On 04/05/2017 09:11 PM, Vasiliy Tolstov wrote: 2017-04-05 17:53 GMT+03:00 Andrea Bolognani : That is true as long as you stick with the public API. If you start messing with the internal RPC and a change in libvirt breaks your code, you get to keep both pieces. I think in case of internal rpc i need to monitor each libvirt release and check git diff or src/remote/remote_protocol.x That is not going to be enough. The RPC protocol is spread among other files too. You really should not hook into our RPC protocol. My question is still unanswered though - why do you need it anyway? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 07:32:59PM +0800, Eli Qiao wrote: > This patch is based on Martin's cache branch. > > This patch amends the cache bank capability as follow: > > > > > > > This is still wrong per my previous comments. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 06:56:09PM +0800, Eli Qiao wrote:
On Thursday, 6 April 2017 at 5:49 PM, Martin Kletzander wrote:
On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote:
> This patch is based on Martin's cache branch.
>
> This patch amends the cache bank capability as follow:
>
>
>
>
>
>
Were we exposing the number of CLoS IDs before? Was there a discussion
about it? Do we want to expose them? Probably yes, I'm just wondering.
Just saw Daniel’s comments, we may need it, with ‘nallocations’, is that okay?
Do you want me to do a reversion now?
> Along with vircaps2xmltest case updated.
>
> Signed-off-by: Eli Qiao mailto:liyong.q...@intel.com)>
> ---
> src/conf/capabilities.c | 112 +--
> src/conf/capabilities.h | 13 ++-
> tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +
> tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 2 +
> tests/vircaps2xmltest.c | 11 +++
> 5 files changed, 132 insertions(+), 8 deletions(-)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 416dd1a..75c0bec 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -52,6 +52,7 @@
> #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
>
> #define SYSFS_SYSTEM_PATH "/sys/devices/system/"
> +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
>
> VIR_LOG_INIT("conf.capabilities")
>
> @@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> virCapsHostCacheBankPtr *caches)
> {
> size_t i = 0;
> + size_t j = 0;
>
> if (!ncaches)
> return 0;
> @@ -900,6 +902,18 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> bank->size >> (kilos * 10),
> kilos ? "KiB" : "B",
> cpus_str);
> + virBufferAdjustIndent(buf, 2);
> + for (j = 0; j < bank->ncontrol; j++) {
> + bool min_kilos = !(bank->controls[j]->min % 1024);
> + virBufferAsprintf(buf,
> + " + "type='%s' nclos='%u'/>\n",
> + bank->controls[j]->min >> (min_kilos * 10),
> + min_kilos ? "KiB" : "B",
> + virCacheTypeToString(bank->controls[j]->type),
> + bank->controls[j]->nclos);
> + }
> + virBufferAdjustIndent(buf, -2);
>
> VIR_FREE(cpus_str);
> }
> @@ -1516,10 +1530,93 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
> if (!ptr)
> return;
>
> + size_t i;
Upstream frowns upon C99 initialization in the middle of the code.
Okay, I can more it before return.
> virBitmapFree(ptr->cpus);
> + for (i = 0; i < ptr->ncontrol; i++)
> + VIR_FREE(ptr->controls[i]);
> + VIR_FREE(ptr->controls);
> VIR_FREE(ptr);
> }
>
> +/* test which kinds of cache control supported
> + * -1: don't support
> + * 0: cat
> + * 1: cdp
> + */
> +static int
> +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> +{
> + int ret = -1;
> + char *path = NULL;
> + if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
> + return -1;
> +
> + if (virFileExists(path)) {
> + ret = 0;
> + } else {
> + VIR_FREE(path);
> + if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level) < 0)
> + return -1;
> + if (virFileExists(path))
> + ret = 1;
> + }
> +
> + VIR_FREE(path);
> + return ret;
> +}
> +
> +static int
> +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char*
type)
> +{
> + int ret = -1;
> + char *path = NULL;
> + char *cbm_mask = NULL;
> + virCapsHostCacheControlPtr control;
> +
> + if (VIR_ALLOC(control) < 0)
> + goto cleanup;
> +
> + if (virFileReadValueUint(&control->nclos,
> + SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
> + bank->level,
> + type) < 0)
> + goto cleanup;
> +
> + if (virFileReadValueString(&cbm_mask,
> + SYSFS_RESCTRL_PATH
> + "info/L%u%s/cbm_mask",
> + bank->level,
> + type) < 0)
> + goto cleanup;
> +
> + virStringTrimOptionalNewline(cbm_mask);
> +
> + control->min = bank->size / (strlen(cbm_mask) * 4);
> +
> + if (STREQ("", type))
> + control->type = VIR_CACHE_TYPE_UNIFIED;
> + else if (STREQ("CODE", type))
> + control->type = VIR_CACHE_TYPE_INSTRUCTION;
> + else if (STREQ("DATA", type))
> + control->type = VIR_CACHE_TYPE_DATA;
> + else
> + goto cleanup;
> +
> + if (VIR_APPEND_ELEMENT(bank->controls,
> + bank->ncontrol,
> + control) < 0)
> + goto error;
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(path);
> + return ret;
> + error:
> + VIR_FREE(control);
> + return -1;
>
The return value is not used anywhere.
yes, I think we can ignore this value or not? if we enabled resctrl (we can
always read correct thing from /sys/fs/resctrl/info/…)
We need to make sure we don't continue and report wrong information.
Like when VIR_APPEND_ELEMENT fails, you must report an error and fail
loading.
> +}
> +
> int
> virCapabilitiesInitCaches(virCapsPtr caps)
> {
> @@ -1533,11 +1630,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> struct dirent *ent = NULL;
> virCapsHostCacheBankPtr bank = NULL;
>
> - /* Minimum level to expose in capabilities. Can be lowered or removed (with
> - * the appropriate code below), but should not be increased, because we'd
> - * lose information. */
> - const int cache_min_level = 3;
> -
>
You are removing this and only reporting
[libvirt] [PATCH 5/7] storage: Introduce virStoragePoolObjGetNames
Mostly code motion to move storageConnectList[Defined]StoragePools
and similar test driver code into virstorageobj.c and rename to
virStoragePoolObjGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan
---
src/conf/virstorageobj.c | 37
src/conf/virstorageobj.h | 8 ++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 58 +++-
src/test/test_driver.c | 48 +---
5 files changed, 72 insertions(+), 80 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index f0201aa..a9367a2 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -580,6 +580,43 @@
virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
}
+int
+virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter,
+ char **const names,
+ int maxnames)
+{
+int nnames = 0;
+size_t i;
+
+for (i = 0; i < pools->count && nnames < maxnames; i++) {
+virStoragePoolObjPtr obj = pools->objs[i];
+virStoragePoolObjLock(obj);
+if (!aclfilter || aclfilter(conn, obj->def)) {
+if ((wantActive && virStoragePoolObjIsActive(obj)) ||
+(!wantActive && !virStoragePoolObjIsActive(obj))) {
+if (VIR_STRDUP(names[nnames++], obj->def->name) < 0) {
+virStoragePoolObjUnlock(obj);
+goto failure;
+}
+}
+}
+virStoragePoolObjUnlock(obj);
+}
+
+return nnames;
+
+ failure:
+while (--nnames >= 0)
+VIR_FREE(names[nnames]);
+memset(names, 0, maxnames * sizeof(*names));
+
+return -1;
+}
+
+
/*
* virStoragePoolObjIsDuplicate:
* @doms : virStoragePoolObjListPtr to search
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 5eeda1e..b651865 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -155,6 +155,14 @@
virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
bool wantActive,
virStoragePoolObjListACLFilter aclfilter);
+int
+virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter,
+ char **const names,
+ int maxnames);
+
void
virStoragePoolObjFree(virStoragePoolObjPtr pool);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7bc3bc4..233110a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -996,6 +996,7 @@ virStoragePoolObjClearVols;
virStoragePoolObjDeleteDef;
virStoragePoolObjFindByName;
virStoragePoolObjFindByUUID;
+virStoragePoolObjGetNames;
virStoragePoolObjIsDuplicate;
virStoragePoolObjListExport;
virStoragePoolObjListFree;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 065380b..9493da8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -490,40 +490,25 @@ storageConnectNumOfStoragePools(virConnectPtr conn)
return nactive;
}
+
static int
storageConnectListStoragePools(virConnectPtr conn,
char **const names,
- int nnames)
+ int maxnames)
{
int got = 0;
-size_t i;
+
+memset(names, 0, maxnames * sizeof(*names));
if (virConnectListStoragePoolsEnsureACL(conn) < 0)
return -1;
storageDriverLock();
-for (i = 0; i < driver->pools.count && got < nnames; i++) {
-virStoragePoolObjPtr obj = driver->pools.objs[i];
-virStoragePoolObjLock(obj);
-if (virConnectListStoragePoolsCheckACL(conn, obj->def) &&
-virStoragePoolObjIsActive(obj)) {
-if (VIR_STRDUP(names[got], obj->def->name) < 0) {
-virStoragePoolObjUnlock(obj);
-goto cleanup;
-}
-got++;
-}
-virStoragePoolObjUnlock(obj);
-}
+got = virStoragePoolObjGetNames(&driver->pools, conn, true,
+virConnectListStoragePoolsCheckACL,
+names, maxnames);
storageDriverUnlock();
return got;
-
- cleanup:
-storageDriverUnlock();
-for (i = 0; i < got; i++)
-VIR_FREE(names[i]);
-memset(names, 0, nnames * sizeof(*names));
-return -1;
}
static int
@@ -542,40 +527,25 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
return nactive;
}
+
static int
storageConnectList
[libvirt] [PATCH 6/7] storage: Pass driver arg by ref
Alter virStoragePoolObjListExport in order to pass the drivers->pools
by reference
Signed-off-by: John Ferlan
---
src/conf/virstorageobj.c | 8
src/conf/virstorageobj.h | 2 +-
src/storage/storage_driver.c | 2 +-
src/test/test_driver.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index a9367a2..34a3225 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1100,7 +1100,7 @@ virStoragePoolMatch(virStoragePoolObjPtr poolobj,
int
virStoragePoolObjListExport(virConnectPtr conn,
-virStoragePoolObjList poolobjs,
+virStoragePoolObjListPtr poolobjs,
virStoragePoolPtr **pools,
virStoragePoolObjListFilter filter,
unsigned int flags)
@@ -,11 +,11 @@ virStoragePoolObjListExport(virConnectPtr conn,
int ret = -1;
size_t i;
-if (pools && VIR_ALLOC_N(tmp_pools, poolobjs.count + 1) < 0)
+if (pools && VIR_ALLOC_N(tmp_pools, poolobjs->count + 1) < 0)
goto cleanup;
-for (i = 0; i < poolobjs.count; i++) {
-virStoragePoolObjPtr poolobj = poolobjs.objs[i];
+for (i = 0; i < poolobjs->count; i++) {
+virStoragePoolObjPtr poolobj = poolobjs->objs[i];
virStoragePoolObjLock(poolobj);
if ((!filter || filter(conn, poolobj->def)) &&
virStoragePoolMatch(poolobj, flags)) {
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index b651865..7c11910 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -191,7 +191,7 @@ virStoragePoolObjUnlock(virStoragePoolObjPtr obj);
int
virStoragePoolObjListExport(virConnectPtr conn,
-virStoragePoolObjList poolobjs,
+virStoragePoolObjListPtr poolobjs,
virStoragePoolPtr **pools,
virStoragePoolObjListFilter filter,
unsigned int flags);
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9493da8..6b8bbb5 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2653,7 +2653,7 @@ storageConnectListAllStoragePools(virConnectPtr conn,
goto cleanup;
storageDriverLock();
-ret = virStoragePoolObjListExport(conn, driver->pools, pools,
+ret = virStoragePoolObjListExport(conn, &driver->pools, pools,
virConnectListAllStoragePoolsCheckACL,
flags);
storageDriverUnlock();
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 1a6601e..a2a17ef 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4194,7 +4194,7 @@ testConnectListAllStoragePools(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1);
testDriverLock(privconn);
-ret = virStoragePoolObjListExport(conn, privconn->pools, pools,
+ret = virStoragePoolObjListExport(conn, &privconn->pools, pools,
NULL, flags);
testDriverUnlock(privconn);
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] storage: Introduce virStoragePoolObjNumOfStoragePools
Unify the NumOf[Defined]StoragePools API into virstorageobj.c from
storage_driver and test_driver. The only real difference between the
two is the test driver doesn't call using the aclfilter API.
Signed-off-by: John Ferlan
---
src/conf/virstorageobj.c | 24
src/conf/virstorageobj.h | 9 +
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 22 --
src/test/test_driver.c | 17 ++---
5 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 2484517..f0201aa 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -556,6 +556,30 @@ virStoragePoolObjDeleteDef(virStoragePoolObjPtr pool)
}
+int
+virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter)
+{
+int npools = 0;
+size_t i;
+
+for (i = 0; i < pools->count; i++) {
+virStoragePoolObjPtr obj = pools->objs[i];
+virStoragePoolObjLock(obj);
+if (!aclfilter || aclfilter(conn, obj->def)) {
+if ((wantActive && virStoragePoolObjIsActive(obj)) ||
+(!wantActive && !virStoragePoolObjIsActive(obj)))
+npools++;
+}
+virStoragePoolObjUnlock(obj);
+}
+
+return npools;
+}
+
+
/*
* virStoragePoolObjIsDuplicate:
* @doms : virStoragePoolObjListPtr to search
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 72daa5a..5eeda1e 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -146,6 +146,15 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
int
virStoragePoolObjDeleteDef(virStoragePoolObjPtr pool);
+typedef bool (*virStoragePoolObjListACLFilter)(virConnectPtr conn,
+ virStoragePoolDefPtr def);
+
+int
+virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter);
+
void
virStoragePoolObjFree(virStoragePoolObjPtr pool);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6298019..7bc3bc4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1002,6 +1002,7 @@ virStoragePoolObjListFree;
virStoragePoolObjLoadAllConfigs;
virStoragePoolObjLoadAllState;
virStoragePoolObjLock;
+virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 475e332..065380b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -477,21 +477,14 @@ storagePoolLookupByVolume(virStorageVolPtr vol)
static int
storageConnectNumOfStoragePools(virConnectPtr conn)
{
-size_t i;
int nactive = 0;
if (virConnectNumOfStoragePoolsEnsureACL(conn) < 0)
return -1;
storageDriverLock();
-for (i = 0; i < driver->pools.count; i++) {
-virStoragePoolObjPtr obj = driver->pools.objs[i];
-virStoragePoolObjLock(obj);
-if (virConnectNumOfStoragePoolsCheckACL(conn, obj->def) &&
-virStoragePoolObjIsActive(obj))
-nactive++;
-virStoragePoolObjUnlock(obj);
-}
+nactive = virStoragePoolObjNumOfStoragePools(&driver->pools, conn, true,
+
virConnectNumOfStoragePoolsCheckACL);
storageDriverUnlock();
return nactive;
@@ -536,21 +529,14 @@ storageConnectListStoragePools(virConnectPtr conn,
static int
storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
{
-size_t i;
int nactive = 0;
if (virConnectNumOfDefinedStoragePoolsEnsureACL(conn) < 0)
return -1;
storageDriverLock();
-for (i = 0; i < driver->pools.count; i++) {
-virStoragePoolObjPtr obj = driver->pools.objs[i];
-virStoragePoolObjLock(obj);
-if (virConnectNumOfDefinedStoragePoolsCheckACL(conn, obj->def) &&
-!virStoragePoolObjIsActive(obj))
-nactive++;
-virStoragePoolObjUnlock(obj);
-}
+nactive = virStoragePoolObjNumOfStoragePools(&driver->pools, conn, false,
+
virConnectNumOfDefinedStoragePoolsCheckACL);
storageDriverUnlock();
return nactive;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 44bd47c..4fb9915 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4115,17 +4115,16 @@ testStoragePoolLookupByVolume(virStorageVolPtr vol)
return testStoragePoolLookupByName(vol->conn, vol->pool);
}
+
static int
testConnectNumOfStoragePools(virConnectPtr conn)
[libvirt] [PATCH 7/7] storage: Create helpers to perform FindByUUID and FindByName
Create a couple of helpers that will perform the same call sequence.
Signed-off-by: John Ferlan
---
src/storage/storage_driver.c | 144 ++-
1 file changed, 59 insertions(+), 85 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6b8bbb5..8b55acc 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -387,6 +387,56 @@ storageStateCleanup(void)
}
+static virStoragePoolObjPtr
+storagePoolObjFindByUUID(const unsigned char *uuid,
+ const char *name)
+{
+virStoragePoolObjPtr pool;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, uuid))) {
+virUUIDFormat(uuid, uuidstr);
+if (name)
+virReportError(VIR_ERR_NO_STORAGE_POOL,
+ _("no storage pool with matching uuid '%s' (%s)"),
+ uuidstr, name);
+else
+virReportError(VIR_ERR_NO_STORAGE_POOL,
+ _("no storage pool with matching uuid '%s'"),
+ uuidstr);
+}
+
+return pool;
+}
+
+
+static virStoragePoolObjPtr
+virStoragePoolObjFromStoragePool(virStoragePoolPtr pool)
+{
+virStoragePoolObjPtr ret;
+
+storageDriverLock();
+ret = storagePoolObjFindByUUID(pool->uuid, pool->name);
+storageDriverUnlock();
+
+return ret;
+}
+
+
+static virStoragePoolObjPtr
+storagePoolObjFindByName(const char *name)
+{
+virStoragePoolObjPtr pool;
+
+storageDriverLock();
+if (!(pool = virStoragePoolObjFindByName(&driver->pools, name)))
+virReportError(VIR_ERR_NO_STORAGE_POOL,
+ _("no storage pool with matching name '%s'"), name);
+storageDriverUnlock();
+
+return pool;
+}
+
static virStoragePoolPtr
storagePoolLookupByUUID(virConnectPtr conn,
@@ -396,16 +446,10 @@ storagePoolLookupByUUID(virConnectPtr conn,
virStoragePoolPtr ret = NULL;
storageDriverLock();
-pool = virStoragePoolObjFindByUUID(&driver->pools, uuid);
+pool = storagePoolObjFindByUUID(uuid, NULL);
storageDriverUnlock();
-
-if (!pool) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(uuid, uuidstr);
-virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'"), uuidstr);
+if (!pool)
return NULL;
-}
if (virStoragePoolLookupByUUIDEnsureACL(conn, pool->def) < 0)
goto cleanup;
@@ -425,15 +469,8 @@ storagePoolLookupByName(virConnectPtr conn,
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
-storageDriverLock();
-pool = virStoragePoolObjFindByName(&driver->pools, name);
-storageDriverUnlock();
-
-if (!pool) {
-virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching name '%s'"), name);
+if (!(pool = storagePoolObjFindByName(name)))
return NULL;
-}
if (virStoragePoolLookupByNameEnsureACL(conn, pool->def) < 0)
goto cleanup;
@@ -452,16 +489,8 @@ storagePoolLookupByVolume(virStorageVolPtr vol)
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
-storageDriverLock();
-pool = virStoragePoolObjFindByName(&driver->pools, vol->pool);
-storageDriverUnlock();
-
-if (!pool) {
-virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching name '%s'"),
- vol->pool);
+if (!(pool = storagePoolObjFindByName(vol->pool)))
return NULL;
-}
if (virStoragePoolLookupByVolumeEnsureACL(vol->conn, pool->def) < 0)
goto cleanup;
@@ -588,25 +617,6 @@ storageConnectFindStoragePoolSources(virConnectPtr conn,
}
-static virStoragePoolObjPtr
-virStoragePoolObjFromStoragePool(virStoragePoolPtr pool)
-{
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virStoragePoolObjPtr ret;
-
-storageDriverLock();
-if (!(ret = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid))) {
-virUUIDFormat(pool->uuid, uuidstr);
-virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s' (%s)"),
- uuidstr, pool->name);
-}
-storageDriverUnlock();
-
-return ret;
-}
-
-
static int storagePoolIsActive(virStoragePoolPtr pool)
{
virStoragePoolObjPtr obj;
@@ -813,14 +823,8 @@ storagePoolUndefine(virStoragePoolPtr obj)
int ret = -1;
storageDriverLock();
-if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) {
-char uuidstr[VIR_UUID_STRING_BUFLEN];
-virUUIDFormat(obj->uuid, uuidstr);
-virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s' (%s)"),
- uuidstr, obj->name);
+if (!(pool = storagePoolObjFindByUUID(obj->uuid, obj->name)))
goto cleanup;
-
[libvirt] [PATCH 3/7] storage: Introduce virStoragePoolObjVolumeListExport
Essentially code motion to move the storage/test driver ListAllVolumes
logic into virstorageobj.c
Signed-off-by: John Ferlan
---
src/conf/virstorageobj.c | 47
src/conf/virstorageobj.h | 7 +++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 37 --
src/test/test_driver.c | 40 -
5 files changed, 63 insertions(+), 69 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 5933618..2484517 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -244,6 +244,53 @@ virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr
volumes,
}
+int
+virStoragePoolObjVolumeListExport(virConnectPtr conn,
+ virStorageVolDefListPtr volumes,
+ virStoragePoolDefPtr pooldef,
+ virStorageVolPtr **vols,
+ virStoragePoolVolumeACLFilter aclfilter)
+{
+int ret = -1;
+size_t i;
+virStorageVolPtr *tmp_vols = NULL;
+virStorageVolPtr vol = NULL;
+int nvols = 0;
+
+/* Just returns the volumes count */
+if (!vols) {
+ret = volumes->count;
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(tmp_vols, volumes->count + 1) < 0)
+goto cleanup;
+
+for (i = 0; i < volumes->count; i++) {
+virStorageVolDefPtr def = volumes->objs[i];
+if (aclfilter && !aclfilter(conn, pooldef, def))
+continue;
+if (!(vol = virGetStorageVol(conn, pooldef->name, def->name, def->key,
+ NULL, NULL)))
+goto cleanup;
+tmp_vols[nvols++] = vol;
+}
+
+*vols = tmp_vols;
+tmp_vols = NULL;
+ret = nvols;
+
+ cleanup:
+if (tmp_vols) {
+for (i = 0; i < nvols; i++)
+virObjectUnref(tmp_vols[i]);
+VIR_FREE(tmp_vols);
+}
+
+return ret;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index bf88094..72daa5a 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -127,6 +127,13 @@ virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr
volumes,
char **const names,
int maxnames);
+int
+virStoragePoolObjVolumeListExport(virConnectPtr conn,
+ virStorageVolDefListPtr volumes,
+ virStoragePoolDefPtr pooldef,
+ virStorageVolPtr **vols,
+ virStoragePoolVolumeACLFilter aclfilter);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a635cac..6298019 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1008,6 +1008,7 @@ virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
virStoragePoolObjUnlock;
virStoragePoolObjVolumeGetNames;
+virStoragePoolObjVolumeListExport;
# cpu/cpu.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ce77fe1..475e332 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1442,16 +1442,13 @@ storagePoolListVolumes(virStoragePoolPtr obj,
return n;
}
+
static int
storagePoolListAllVolumes(virStoragePoolPtr pool,
virStorageVolPtr **vols,
unsigned int flags)
{
virStoragePoolObjPtr obj;
-size_t i;
-virStorageVolPtr *tmp_vols = NULL;
-virStorageVolPtr vol = NULL;
-int nvols = 0;
int ret = -1;
virCheckFlags(0, -1);
@@ -1468,38 +1465,12 @@ storagePoolListAllVolumes(virStoragePoolPtr pool,
goto cleanup;
}
- /* Just returns the volumes count */
-if (!vols) {
-ret = obj->volumes.count;
-goto cleanup;
-}
+ret = virStoragePoolObjVolumeListExport(pool->conn, &obj->volumes,
+obj->def, vols,
+
virStoragePoolListAllVolumesCheckACL);
-if (VIR_ALLOC_N(tmp_vols, obj->volumes.count + 1) < 0)
-goto cleanup;
-
-for (i = 0; i < obj->volumes.count; i++) {
-if (!virStoragePoolListAllVolumesCheckACL(pool->conn, obj->def,
- obj->volumes.objs[i]))
-continue;
-if (!(vol = virGetStorageVol(pool->conn, obj->def->name,
- obj->volumes.objs[i]->name,
- obj->volumes.objs[i]->key,
- NULL, NULL)))
-goto cleanup;
-tmp_vols[n
[libvirt] [PATCH 0/7] storage: Create common virStorageObj* APIs
Merge code in storage_driver.c and test_driver into virstorageobj to
count the number of volumes/pools, to return a list of names, return
the collected list of objects for volumes/pools. For volumes, that's
moved code, while for pools that's just changing the export API to
take an address of 'devobjs' to follow other usages. I also added
code to converge the FindBy{UUID|Name}
FWIW: This is part of the common driver objects code I've been working
through. I figured I will post each driver separately rather than one
John Ferlan (7):
storage: Introduce virStoragePoolObjNumOfVolumes
storage: Introduce virStoragePoolObjVolumeGetNames
storage: Introduce virStoragePoolObjVolumeListExport
storage: Introduce virStoragePoolObjNumOfStoragePools
storage: Introduce virStoragePoolObjGetNames
storage: Pass driver arg by ref
storage: Create helpers to perform FindByUUID and FindByName
src/conf/virstorageobj.c | 165 +++-
src/conf/virstorageobj.h | 45 ++-
src/libvirt_private.syms | 5 +
src/storage/storage_driver.c | 297 +--
src/test/test_driver.c | 131 +--
5 files changed, 332 insertions(+), 311 deletions(-)
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] storage: Introduce virStoragePoolObjVolumeGetNames
Mostly code motion to move storagePoolListVolumes code into virstorageobj.c
and rename to virStoragePoolObjVolumeGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan
---
src/conf/virstorageobj.c | 30 ++
src/conf/virstorageobj.h | 8
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 24 ++--
src/test/test_driver.c | 21 ++---
5 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index e57694c..5933618 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -214,6 +214,36 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr
volumes,
}
+int
+virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+virConnectPtr conn,
+virStoragePoolDefPtr pooldef,
+virStoragePoolVolumeACLFilter aclfilter,
+char **const names,
+int maxnames)
+{
+int nnames = 0;
+size_t i;
+
+for (i = 0; i < volumes->count && nnames < maxnames; i++) {
+virStorageVolDefPtr def = volumes->objs[i];
+if (aclfilter && !aclfilter(conn, pooldef, def))
+continue;
+if (VIR_STRDUP(names[nnames++], def->name) < 0)
+goto failure;
+}
+
+return nnames;
+
+ failure:
+while (--nnames >= 0)
+VIR_FREE(names[nnames]);
+memset(names, 0, maxnames * sizeof(*names));
+
+return -1;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 3effe7a..bf88094 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -119,6 +119,14 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr
volumes,
virStoragePoolDefPtr pooldef,
virStoragePoolVolumeACLFilter aclfilter);
+int
+virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+virConnectPtr conn,
+virStoragePoolDefPtr pooldef,
+virStoragePoolVolumeACLFilter aclfilter,
+char **const names,
+int maxnames);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9580622..a635cac 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1007,6 +1007,7 @@ virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
virStoragePoolObjUnlock;
+virStoragePoolObjVolumeGetNames;
# cpu/cpu.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7d2f74d..ce77fe1 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1411,14 +1411,14 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
return ret;
}
+
static int
storagePoolListVolumes(virStoragePoolPtr obj,
char **const names,
int maxnames)
{
virStoragePoolObjPtr pool;
-size_t i;
-int n = 0;
+int n = -1;
memset(names, 0, maxnames * sizeof(*names));
@@ -1434,24 +1434,12 @@ storagePoolListVolumes(virStoragePoolPtr obj,
goto cleanup;
}
-for (i = 0; i < pool->volumes.count && n < maxnames; i++) {
-if (!virStoragePoolListVolumesCheckACL(obj->conn, pool->def,
- pool->volumes.objs[i]))
-continue;
-if (VIR_STRDUP(names[n++], pool->volumes.objs[i]->name) < 0)
-goto cleanup;
-}
-
-virStoragePoolObjUnlock(pool);
-return n;
-
+n = virStoragePoolObjVolumeGetNames(&pool->volumes, obj->conn, pool->def,
+virStoragePoolListVolumesCheckACL,
+names, maxnames);
cleanup:
virStoragePoolObjUnlock(pool);
-for (n = 0; n < maxnames; n++)
-VIR_FREE(names[n]);
-
-memset(names, 0, maxnames * sizeof(*names));
-return -1;
+return n;
}
static int
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a4b5833..b2840fa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4830,6 +4830,7 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool)
return ret;
}
+
static int
testStoragePoolListVolumes(virStoragePoolPtr pool,
char **const names,
@@ -4837,8 +4838,7 @@ testStoragePoolListVolumes(virStoragePoolPtr pool,
{
testDriverPtr privconn = pool->conn->privateData;
[libvirt] [PATCH 1/7] storage: Introduce virStoragePoolObjNumOfVolumes
Unify the NumOfVolumes API into virstorageobj.c from storage_driver and
test_driver. The only real difference between the two is the test driver
doesn't call using the aclfilter API.
Signed-off-by: John Ferlan
---
src/conf/virstorageobj.c | 19 +++
src/conf/virstorageobj.h | 11 +++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 10 +++---
src/test/test_driver.c | 3 ++-
5 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 34f2eb7..e57694c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -195,6 +195,25 @@ virStorageVolDefFindByName(virStoragePoolObjPtr pool,
}
+int
+virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter)
+{
+int nvolumes = 0;
+size_t i;
+
+for (i = 0; i < volumes->count; i++) {
+virStorageVolDefPtr def = volumes->objs[i];
+if (aclfilter && aclfilter(conn, pooldef, def))
+nvolumes++;
+}
+
+return nvolumes;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index ecba94e..3effe7a 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -108,6 +108,17 @@ virStorageVolDefFindByName(virStoragePoolObjPtr pool,
void
virStoragePoolObjClearVols(virStoragePoolObjPtr pool);
+typedef bool
+(*virStoragePoolVolumeACLFilter)(virConnectPtr conn,
+ virStoragePoolDefPtr pool,
+ virStorageVolDefPtr def);
+
+int
+virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92083e5..9580622 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1002,6 +1002,7 @@ virStoragePoolObjListFree;
virStoragePoolObjLoadAllConfigs;
virStoragePoolObjLoadAllState;
virStoragePoolObjLock;
+virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index fea7698..7d2f74d 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1390,7 +1390,6 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
{
virStoragePoolObjPtr pool;
int ret = -1;
-size_t i;
if (!(pool = virStoragePoolObjFromStoragePool(obj)))
return -1;
@@ -1403,12 +1402,9 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
_("storage pool '%s' is not active"), pool->def->name);
goto cleanup;
}
-ret = 0;
-for (i = 0; i < pool->volumes.count; i++) {
-if (virStoragePoolNumOfVolumesCheckACL(obj->conn, pool->def,
- pool->volumes.objs[i]))
-ret++;
-}
+
+ret = virStoragePoolObjNumOfVolumes(&pool->volumes, obj->conn, pool->def,
+virStoragePoolNumOfVolumesCheckACL);
cleanup:
virStoragePoolObjUnlock(pool);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cce4d2d..a4b5833 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4821,7 +4821,8 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool)
goto cleanup;
}
-ret = privpool->volumes.count;
+ret = virStoragePoolObjNumOfVolumes(&privpool->volumes, pool->conn,
+privpool->def, NULL);
cleanup:
if (privpool)
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] nwfilter: Introduce virNWFilterObjNumOfNWFilters
Mostly code motion from nwfilter_driver to virnwfilterobj with one caveat
to add the virNWFilterObjListFilter typedef and pass it as an 'aclfilter'
argument to allow for future possible test driver adjustments to count
the number of filters (similar to how node device has done this).
Signed-off-by: John Ferlan
---
src/conf/virnwfilterobj.c | 20
src/conf/virnwfilterobj.h | 9 +
src/libvirt_private.syms | 1 +
src/nwfilter/nwfilter_driver.c | 15 ++-
4 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 7e13afb..86d77ce 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -305,6 +305,26 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
}
+int
+virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr nwfilters,
+ virConnectPtr conn,
+ virNWFilterObjListFilter aclfilter)
+{
+size_t i;
+int nfilters = 0;
+
+for (i = 0; i < nwfilters->count; i++) {
+virNWFilterObjPtr obj = nwfilters->objs[i];
+virNWFilterObjLock(obj);
+if (aclfilter && aclfilter(conn, obj->def))
+nfilters++;
+virNWFilterObjUnlock(obj);
+}
+
+return nfilters;
+}
+
+
static virNWFilterObjPtr
virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters,
const char *configDir,
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index 1d80455..a8ecc35 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -81,6 +81,15 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters,
int
virNWFilterObjTestUnassignDef(virNWFilterObjPtr nwfilter);
+typedef bool
+(*virNWFilterObjListFilter)(virConnectPtr conn,
+virNWFilterDefPtr def);
+
+int
+virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr nwfilters,
+ virConnectPtr conn,
+ virNWFilterObjListFilter aclfilter);
+
int
virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters,
const char *configDir);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92083e5..706b2b4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -962,6 +962,7 @@ virNWFilterObjFindByUUID;
virNWFilterObjListFree;
virNWFilterObjLoadAllConfigs;
virNWFilterObjLock;
+virNWFilterObjNumOfNWFilters;
virNWFilterObjRemove;
virNWFilterObjTestUnassignDef;
virNWFilterObjUnlock;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 4ea216a..70bdea2 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -414,22 +414,11 @@ nwfilterLookupByName(virConnectPtr conn,
static int
nwfilterConnectNumOfNWFilters(virConnectPtr conn)
{
-size_t i;
-int n;
-
if (virConnectNumOfNWFiltersEnsureACL(conn) < 0)
return -1;
-n = 0;
-for (i = 0; i < driver->nwfilters.count; i++) {
-virNWFilterObjPtr obj = driver->nwfilters.objs[i];
-virNWFilterObjLock(obj);
-if (virConnectNumOfNWFiltersCheckACL(conn, obj->def))
-n++;
-virNWFilterObjUnlock(obj);
-}
-
-return n;
+return virNWFilterObjNumOfNWFilters(&driver->nwfilters, conn,
+virConnectNumOfNWFiltersCheckACL);
}
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] nwfilter: Create common virNWFilterObj* APIs
Effectively move code from nwfilter_driver.c into virnwfiltereobj to count the number of devices, to return a list of names, and to return an array of filters. FWIW: This is part of the common driver objects code I've been working through. I figured I will post each driver separately rather than one larger series. John Ferlan (3): nwfilter: Introduce virNWFilterObjNumOfNWFilters nwfilter: Introduce virNWFilterObjGetNames nwfilter: Introduce virNWFilterObjListExport src/conf/virnwfilterobj.c | 104 + src/conf/virnwfilterobj.h | 22 + src/libvirt_private.syms | 3 ++ src/nwfilter/nwfilter_driver.c | 86 +- 4 files changed, 140 insertions(+), 75 deletions(-) -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] nwfilter: Introduce virNWFilterObjGetNames
Mostly code motion to move nwfilterConnectListNWFilters into nwfilterobj.c
and rename to virNWFilterObjGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan
---
src/conf/virnwfilterobj.c | 34 ++
src/conf/virnwfilterobj.h | 7 +++
src/libvirt_private.syms | 1 +
src/nwfilter/nwfilter_driver.c | 29 ++---
4 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 86d77ce..66593b8 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -325,6 +325,40 @@ virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr
nwfilters,
}
+int
+virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters,
+ virConnectPtr conn,
+ virNWFilterObjListFilter aclfilter,
+ char **const names,
+ int maxnames)
+{
+int nnames = 0;
+size_t i;
+
+for (i = 0; i < nwfilters->count && nnames < maxnames; i++) {
+virNWFilterObjPtr obj = nwfilters->objs[i];
+virNWFilterObjLock(obj);
+if (aclfilter && aclfilter(conn, obj->def)) {
+if (VIR_STRDUP(names[nnames], obj->def->name) < 0) {
+virNWFilterObjUnlock(obj);
+goto failure;
+}
+nnames++;
+}
+virNWFilterObjUnlock(obj);
+}
+
+return nnames;
+
+ failure:
+while (--nnames >= 0)
+VIR_FREE(names[nnames]);
+memset(names, 0, maxnames * sizeof(*names));
+
+return -1;
+}
+
+
static virNWFilterObjPtr
virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters,
const char *configDir,
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index a8ecc35..cd1600c 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -91,6 +91,13 @@ virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr nwfilters,
virNWFilterObjListFilter aclfilter);
int
+virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters,
+ virConnectPtr conn,
+ virNWFilterObjListFilter aclfilter,
+ char **const names,
+ int maxnames);
+
+int
virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters,
const char *configDir);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 706b2b4..fa1219f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -959,6 +959,7 @@ virNodeDeviceObjUnlock;
virNWFilterObjAssignDef;
virNWFilterObjFindByName;
virNWFilterObjFindByUUID;
+virNWFilterObjGetNames;
virNWFilterObjListFree;
virNWFilterObjLoadAllConfigs;
virNWFilterObjLock;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 70bdea2..0d164a2 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -425,36 +425,19 @@ nwfilterConnectNumOfNWFilters(virConnectPtr conn)
static int
nwfilterConnectListNWFilters(virConnectPtr conn,
char **const names,
- int nnames)
+ int maxnames)
{
-int got = 0;
-size_t i;
+int nnames;
if (virConnectListNWFiltersEnsureACL(conn) < 0)
return -1;
nwfilterDriverLock();
-for (i = 0; i < driver->nwfilters.count && got < nnames; i++) {
-virNWFilterObjPtr obj = driver->nwfilters.objs[i];
-virNWFilterObjLock(obj);
-if (virConnectListNWFiltersCheckACL(conn, obj->def)) {
-if (VIR_STRDUP(names[got], obj->def->name) < 0) {
-virNWFilterObjUnlock(obj);
-goto cleanup;
-}
-got++;
-}
-virNWFilterObjUnlock(obj);
-}
-nwfilterDriverUnlock();
-return got;
-
- cleanup:
+nnames = virNWFilterObjGetNames(&driver->nwfilters, conn,
+virConnectListNWFiltersCheckACL,
+names, maxnames);
nwfilterDriverUnlock();
-for (i = 0; i < got; i++)
-VIR_FREE(names[i]);
-memset(names, 0, nnames * sizeof(*names));
-return -1;
+return nnames;
}
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] nwfilter: Introduce virNWFilterObjListExport
Essentially code motion to move the ListExport function from nwfilter_driver
into virnwfilterobj
Signed-off-by: John Ferlan
---
src/conf/virnwfilterobj.c | 50 ++
src/conf/virnwfilterobj.h | 6 +
src/libvirt_private.syms | 1 +
src/nwfilter/nwfilter_driver.c | 42 +++
4 files changed, 60 insertions(+), 39 deletions(-)
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 66593b8..e550b22 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -359,6 +359,56 @@ virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters,
}
+int
+virNWFilterObjListExport(virConnectPtr conn,
+ virNWFilterObjListPtr nwfilters,
+ virNWFilterPtr **filters,
+ virNWFilterObjListFilter aclfilter)
+{
+virNWFilterPtr *tmp_filters = NULL;
+int nfilters = 0;
+virNWFilterPtr filter = NULL;
+virNWFilterObjPtr obj = NULL;
+size_t i;
+int ret = -1;
+
+if (!filters) {
+ret = nwfilters->count;
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(tmp_filters, nwfilters->count + 1) < 0)
+goto cleanup;
+
+for (i = 0; i < nwfilters->count; i++) {
+obj = nwfilters->objs[i];
+virNWFilterObjLock(obj);
+if (aclfilter && aclfilter(conn, obj->def)) {
+if (!(filter = virGetNWFilter(conn, obj->def->name,
+ obj->def->uuid))) {
+virNWFilterObjUnlock(obj);
+goto cleanup;
+}
+tmp_filters[nfilters++] = filter;
+}
+virNWFilterObjUnlock(obj);
+}
+
+*filters = tmp_filters;
+tmp_filters = NULL;
+ret = nfilters;
+
+ cleanup:
+if (tmp_filters) {
+for (i = 0; i < nfilters; i ++)
+virObjectUnref(tmp_filters[i]);
+}
+VIR_FREE(tmp_filters);
+
+return ret;
+}
+
+
static virNWFilterObjPtr
virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters,
const char *configDir,
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index cd1600c..49b1170 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -98,6 +98,12 @@ virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters,
int maxnames);
int
+virNWFilterObjListExport(virConnectPtr conn,
+ virNWFilterObjListPtr nwfilters,
+ virNWFilterPtr **filters,
+ virNWFilterObjListFilter aclfilter);
+
+int
virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters,
const char *configDir);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fa1219f..8ecf9d6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -960,6 +960,7 @@ virNWFilterObjAssignDef;
virNWFilterObjFindByName;
virNWFilterObjFindByUUID;
virNWFilterObjGetNames;
+virNWFilterObjListExport;
virNWFilterObjListFree;
virNWFilterObjLoadAllConfigs;
virNWFilterObjLock;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 0d164a2..f6c419c 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -446,12 +446,7 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn,
virNWFilterPtr **filters,
unsigned int flags)
{
-virNWFilterPtr *tmp_filters = NULL;
-int nfilters = 0;
-virNWFilterPtr filter = NULL;
-virNWFilterObjPtr obj = NULL;
-size_t i;
-int ret = -1;
+int ret;
virCheckFlags(0, -1);
@@ -459,40 +454,9 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn,
return -1;
nwfilterDriverLock();
-
-if (!filters) {
-ret = driver->nwfilters.count;
-goto cleanup;
-}
-
-if (VIR_ALLOC_N(tmp_filters, driver->nwfilters.count + 1) < 0)
-goto cleanup;
-
-for (i = 0; i < driver->nwfilters.count; i++) {
-obj = driver->nwfilters.objs[i];
-virNWFilterObjLock(obj);
-if (virConnectListAllNWFiltersCheckACL(conn, obj->def)) {
-if (!(filter = virGetNWFilter(conn, obj->def->name,
- obj->def->uuid))) {
-virNWFilterObjUnlock(obj);
-goto cleanup;
-}
-tmp_filters[nfilters++] = filter;
-}
-virNWFilterObjUnlock(obj);
-}
-
-*filters = tmp_filters;
-tmp_filters = NULL;
-ret = nfilters;
-
- cleanup:
+ret = virNWFilterObjListExport(conn, &driver->nwfilters, filters,
+ virConnectListAllNWFiltersCheckACL);
nwfilterDriverUnlock();
-if (tmp_filters) {
-for (i = 0; i < nfilters; i ++)
-virObjectUnref(tmp_filters[i]);
-}
-VIR_FREE(tmp_filters);
return ret;
}
--
2.9.3
--
libvir-list mailing
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 07:31:00PM +0800, Eli Qiao wrote: > > > On Thursday, 6 April 2017 at 5:51 PM, Daniel P. Berrange wrote: > > > On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: > > > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > > > This patch is based on Martin's cache branch. > > > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > cpus='0-5'/> > > > > > > > > > > > cpus='6-11'/> > > > > > > > > > > > > > > > > > > > > Either the XML is malformed, or the indentation is wrong. The indentation > > suggests you want nested XML elements, but the parent element is an empty > > tag, so you've actually got a flat namespace here. > > > > > > > > > > > > I validate it online, no errors, the result is: You've checked the XML syntax, but *not* the semantics. You've implemented this: but indentation suggests you meant to do both are valid XML, but they have completely different semantics Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/12] util: Add virStringTrimOptionalNewline
On Wed, Apr 05, 2017 at 04:36:26PM +0200, Martin Kletzander wrote:
> And use it in virFileRead*
>
> Signed-off-by: Martin Kletzander
> ---
> src/util/virfile.c| 18 +++---
> src/util/virhostcpu.c | 4 ++--
> src/util/virstring.h | 8
> src/util/virsysfs.c | 2 ++
> 4 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index c0f448d3437d..cbfa3849d793 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3811,7 +3811,6 @@ int
> virFileReadValueInt(const char *path, int *value)
> {
> char *str = NULL;
> -char *endp = NULL;
>
> if (!virFileExists(path))
> return -2;
> @@ -3819,8 +3818,9 @@ virFileReadValueInt(const char *path, int *value)
> if (virFileReadAll(path, INT_STRLEN_BOUND(*value), &str) < 0)
> return -1;
>
> -if (virStrToLong_i(str, &endp, 10, value) < 0 ||
> -(endp && !c_isspace(*endp))) {
> +virStringTrimOptionalNewline(str);
> +
> +if (virStrToLong_i(str, NULL, 10, value) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Invalid integer value '%s' in file '%s'"),
> str, path);
> @@ -3847,7 +3847,6 @@ int
> virFileReadValueUint(const char *path, unsigned int *value)
> {
> char *str = NULL;
> -char *endp = NULL;
>
> if (!virFileExists(path))
> return -2;
> @@ -3855,8 +3854,9 @@ virFileReadValueUint(const char *path, unsigned int
> *value)
> if (virFileReadAll(path, INT_STRLEN_BOUND(*value), &str) < 0)
> return -1;
>
> -if (virStrToLong_uip(str, &endp, 10, value) < 0 ||
> -(endp && !c_isspace(*endp))) {
> +virStringTrimOptionalNewline(str);
> +
> +if (virStrToLong_uip(str, NULL, 10, value)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Invalid unsigned integer value '%s' in file '%s'"),
> str, path);
> @@ -3886,7 +3886,6 @@ virFileReadValueBitmap(const char *path,
> {
> char *buf = NULL;
> int ret = -1;
> -char *tmp = NULL;
>
> if (!virFileExists(path))
> return -2;
> @@ -3894,10 +3893,7 @@ virFileReadValueBitmap(const char *path,
> if (virFileReadAll(path, maxlen, &buf) < 0)
> goto cleanup;
>
> -/* trim optinoal newline at the end */
> -tmp = buf + strlen(buf) - 1;
> -if (*tmp == '\n')
> -*tmp = '\0';
> +virStringTrimOptionalNewline(buf);
>
> *value = virBitmapParseUnlimited(buf);
> if (!*value)
> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index 02b9fc8eb94f..a660e3f4dbe5 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -847,13 +847,13 @@ virHostCPUParseCountLinux(void)
> tmp = str;
> do {
> if (virStrToLong_i(tmp, &tmp, 10, &ret) < 0 ||
> -!strchr(",-\n", *tmp)) {
> +!strchr(",-", *tmp)) {
> virReportError(VIR_ERR_NO_SUPPORT,
> _("failed to parse %s"), str);
> ret = -1;
> goto cleanup;
> }
> -} while (*tmp++ != '\n');
> +} while (*tmp++ && *tmp);
> ret++;
>
> cleanup:
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index a5550e30d2e2..603650aa1588 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -288,4 +288,12 @@ bool virStringBufferIsPrintable(const uint8_t *buf,
> size_t buflen);
>
> char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
>
> +static inline void
> +virStringTrimOptionalNewline(char *str)
> +{
> +char *tmp = str + strlen(str) - 1;
> +if (*tmp == '\n')
> +*tmp = '\0';
> +}
Is there any other reason for using this instead of virTrimSpaces than just
being a bit faster? Because I think the performance gain in this case compared
to 1 iteration of the while loop is very small, thus if possible I would avoid
creating a function for it when there is virTrimSpaces (and I think
virSkipSpacesBackwards would be usable too).
Other than that, it makes perfect sense to me, ACK.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2] Expose resource control capabilites on cache bank
This patch is based on Martin's cache branch.
This patch amends the cache bank capability as follow:
Along with vircaps2xmltest case updated.
Signed-off-by: Eli Qiao
---
src/conf/capabilities.c | 110 +++
src/conf/capabilities.h | 13 ++-
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 2 +
tests/vircaps2xmltest.c | 9 ++
4 files changed, 132 insertions(+), 2 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 416dd1a..3ea518e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -52,6 +52,7 @@
#define VIR_FROM_THIS VIR_FROM_CAPABILITIES
#define SYSFS_SYSTEM_PATH "/sys/devices/system/"
+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
VIR_LOG_INIT("conf.capabilities")
@@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
virCapsHostCacheBankPtr *caches)
{
size_t i = 0;
+size_t j = 0;
if (!ncaches)
return 0;
@@ -900,6 +902,18 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
bank->size >> (kilos * 10),
kilos ? "KiB" : "B",
cpus_str);
+virBufferAdjustIndent(buf, 2);
+for (j = 0; j < bank->ncontrol; j++) {
+bool min_kilos = !(bank->controls[j]->min % 1024);
+virBufferAsprintf(buf,
+ "\n",
+ bank->controls[j]->min >> (min_kilos * 10),
+ min_kilos ? "KiB" : "B",
+ virCacheTypeToString(bank->controls[j]->type),
+ bank->controls[j]->nallocations);
+}
+virBufferAdjustIndent(buf, -2);
VIR_FREE(cpus_str);
}
@@ -1513,13 +1527,97 @@ virCapsHostCacheBankEquals(virCapsHostCacheBankPtr a,
void
virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
{
+size_t i;
+
if (!ptr)
return;
virBitmapFree(ptr->cpus);
+for (i = 0; i < ptr->ncontrol; i++)
+VIR_FREE(ptr->controls[i]);
+VIR_FREE(ptr->controls);
VIR_FREE(ptr);
}
+/* test which kinds of cache control supported
+ * -1: don't support
+ * 0: cat
+ * 1: cdp
+ */
+static int
+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
+{
+int ret = -1;
+char *path = NULL;
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
+return -1;
+
+if (virFileExists(path)) {
+ret = 0;
+} else {
+VIR_FREE(path);
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level)
< 0)
+return -1;
+if (virFileExists(path))
+ret = 1;
+}
+
+VIR_FREE(path);
+return ret;
+}
+
+static int
+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char* type)
+{
+int ret = -1;
+char *path = NULL;
+char *cbm_mask = NULL;
+virCapsHostCacheControlPtr control;
+
+if (VIR_ALLOC(control) < 0)
+goto cleanup;
+
+if (virFileReadValueUint(&control->nallocations,
+ SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+if (virFileReadValueString(&cbm_mask,
+ SYSFS_RESCTRL_PATH
+ "info/L%u%s/cbm_mask",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+virStringTrimOptionalNewline(cbm_mask);
+
+control->min = bank->size / (strlen(cbm_mask) * 4);
+
+if (STREQ("", type))
+control->type = VIR_CACHE_TYPE_UNIFIED;
+else if (STREQ("CODE", type))
+control->type = VIR_CACHE_TYPE_INSTRUCTION;
+else if (STREQ("DATA", type))
+control->type = VIR_CACHE_TYPE_DATA;
+else
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(bank->controls,
+ bank->ncontrol,
+ control) < 0)
+goto error;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+return ret;
+ error:
+VIR_FREE(control);
+return -1;
+}
+
int
virCapabilitiesInitCaches(virCapsPtr caps)
{
@@ -1601,6 +1699,17 @@ virCapabilitiesInitCaches(virCapsPtr caps)
continue;
}
+ret = virCapabilitiesGetCacheControlType(bank);
+
+if (ret == 0) {
+if (virCapabilitiesGetCacheControl(bank, "") < 0)
+goto cleanup;
+} else if (ret == 1) {
+if ((virCapabilitiesGetCacheControl(bank, "CODE") < 0) ||
+(virCapabilitiesGetCacheControl(bank, "DATA") < 0))
+goto cleanup;
+}
+
for (tmp_c = type; *tmp_c != '\0'; tmp_c++)
*tmp_c = c_tolower(*tmp_c);
@@ -1617,6 +1726,7 @@ virCapabiliti
[libvirt] [PATCH 0/3] nodedev: Create common virNodeDeviceObj* APIs
Merge code in node_device_driver.c and test_driver into virnodedeviceobj to count the number of devices and to return a list of names. Also modify the export list API to take an address of 'devobjs' to follow other usages. FWIW: This is part of the common driver objects code I've been working through. I figured I will post each driver separately rather than one larger series. John Ferlan (3): nodedev: Introduce virNodeDeviceObjNumOfDevices nodedev: Introduce virNodeDeviceObjGetNames nodedev: Pass driver arg by ref src/conf/virnodedeviceobj.c | 66 +--- src/conf/virnodedeviceobj.h | 20 --- src/libvirt_private.syms | 3 +- src/node_device/node_device_driver.c | 48 +++--- src/test/test_driver.c | 33 -- 5 files changed, 98 insertions(+), 72 deletions(-) -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] nodedev: Introduce virNodeDeviceObjNumOfDevices
Unify the NumOfDevices API into virnodedeviceobj.c from node_device_driver
and test_driver. The only real difference between the two is that the test
driver doesn't call the aclfilter API.
Signed-off-by: John Ferlan
---
src/conf/virnodedeviceobj.c | 22 ++
src/conf/virnodedeviceobj.h | 6 ++
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 12 ++--
src/test/test_driver.c | 6 +-
5 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 3fe3ae5..f0c20c2 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -474,6 +474,28 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr devobj,
}
+int
+virNodeDeviceObjNumOfDevices(virNodeDeviceObjListPtr devs,
+ virConnectPtr conn,
+ const char *cap,
+ virNodeDeviceObjListFilter aclfilter)
+{
+size_t i;
+int ndevs = 0;
+
+for (i = 0; i < devs->count; i++) {
+virNodeDeviceObjPtr obj = devs->objs[i];
+virNodeDeviceObjLock(obj);
+if (aclfilter && aclfilter(conn, obj->def) &&
+(!cap || virNodeDeviceObjHasCap(obj, cap)))
+++ndevs;
+virNodeDeviceObjUnlock(obj);
+}
+
+return ndevs;
+}
+
+
#define MATCH(FLAG) ((flags & (VIR_CONNECT_LIST_NODE_DEVICES_CAP_ ## FLAG)) &&
\
virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_ ## FLAG))
static bool
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index e32bbf8..7d303a0 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -83,6 +83,12 @@ typedef bool
virNodeDeviceDefPtr def);
int
+virNodeDeviceObjNumOfDevices(virNodeDeviceObjListPtr devs,
+ virConnectPtr conn,
+ const char *cap,
+ virNodeDeviceObjListFilter aclfilter);
+
+int
virNodeDeviceObjListExport(virConnectPtr conn,
virNodeDeviceObjList devobjs,
virNodeDevicePtr **devices,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92083e5..104ed88 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -951,6 +951,7 @@ virNodeDeviceObjHasCap;
virNodeDeviceObjListExport;
virNodeDeviceObjListFree;
virNodeDeviceObjLock;
+virNodeDeviceObjNumOfDevices;
virNodeDeviceObjRemove;
virNodeDeviceObjUnlock;
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index 99f7bc5..f90b168 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -160,7 +160,6 @@ nodeNumOfDevices(virConnectPtr conn,
unsigned int flags)
{
int ndevs = 0;
-size_t i;
if (virNodeNumOfDevicesEnsureACL(conn) < 0)
return -1;
@@ -168,15 +167,8 @@ nodeNumOfDevices(virConnectPtr conn,
virCheckFlags(0, -1);
nodeDeviceLock();
-for (i = 0; i < driver->devs.count; i++) {
-virNodeDeviceObjPtr obj = driver->devs.objs[i];
-virNodeDeviceObjLock(obj);
-if (virNodeNumOfDevicesCheckACL(conn, obj->def) &&
-((cap == NULL) ||
- virNodeDeviceObjHasCap(obj, cap)))
-++ndevs;
-virNodeDeviceObjUnlock(obj);
-}
+ndevs = virNodeDeviceObjNumOfDevices(&driver->devs, conn, cap,
+ virNodeNumOfDevicesCheckACL);
nodeDeviceUnlock();
return ndevs;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cce4d2d..2f1f4fa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5385,15 +5385,11 @@ testNodeNumOfDevices(virConnectPtr conn,
{
testDriverPtr driver = conn->privateData;
int ndevs = 0;
-size_t i;
virCheckFlags(0, -1);
testDriverLock(driver);
-for (i = 0; i < driver->devs.count; i++)
-if ((cap == NULL) ||
-virNodeDeviceObjHasCap(driver->devs.objs[i], cap))
-++ndevs;
+ndevs = virNodeDeviceObjNumOfDevices(&driver->devs, conn, cap, NULL);
testDriverUnlock(driver);
return ndevs;
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] nodedev: Pass driver arg by ref
Alter virNodeDeviceObjListExport in order to pass the drivers->devs
by reference
Signed-off-by: John Ferlan
---
src/conf/virnodedeviceobj.c | 8
src/conf/virnodedeviceobj.h | 2 +-
src/node_device/node_device_driver.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 5a8dfe7..fc82799 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -561,7 +561,7 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
int
virNodeDeviceObjListExport(virConnectPtr conn,
- virNodeDeviceObjList devobjs,
+ virNodeDeviceObjListPtr devobjs,
virNodeDevicePtr **devices,
virNodeDeviceObjListFilter filter,
unsigned int flags)
@@ -572,11 +572,11 @@ virNodeDeviceObjListExport(virConnectPtr conn,
int ret = -1;
size_t i;
-if (devices && VIR_ALLOC_N(tmp_devices, devobjs.count + 1) < 0)
+if (devices && VIR_ALLOC_N(tmp_devices, devobjs->count + 1) < 0)
goto cleanup;
-for (i = 0; i < devobjs.count; i++) {
-virNodeDeviceObjPtr devobj = devobjs.objs[i];
+for (i = 0; i < devobjs->count; i++) {
+virNodeDeviceObjPtr devobj = devobjs->objs[i];
virNodeDeviceObjLock(devobj);
if ((!filter || filter(conn, devobj->def)) &&
virNodeDeviceMatch(devobj, flags)) {
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 0acd8ce..b8b534b 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -94,7 +94,7 @@ virNodeDeviceObjGetNames(virNodeDeviceObjListPtr devs,
int
virNodeDeviceObjListExport(virConnectPtr conn,
- virNodeDeviceObjList devobjs,
+ virNodeDeviceObjListPtr devobjs,
virNodeDevicePtr **devices,
virNodeDeviceObjListFilter filter,
unsigned int flags);
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index e850a22..269b086 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -213,7 +213,7 @@ nodeConnectListAllNodeDevices(virConnectPtr conn,
return -1;
nodeDeviceLock();
-ret = virNodeDeviceObjListExport(conn, driver->devs, devices,
+ret = virNodeDeviceObjListExport(conn, &driver->devs, devices,
virConnectListAllNodeDevicesCheckACL,
flags);
nodeDeviceUnlock();
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] nodedev: Introduce virNodeDeviceObjGetNames
Unify the *ListDevice API into virnodedeviceobj.c from node_device_driver
and test_driver. The only real difference between the two is that the test
driver doesn't call the aclfilter API. The name of the new API follows that
of other drivers to "GetNames".
NB: Change some variable names to match what they really are - consistency
with other drivers. Also added a clear of the input names.
This also allows virNodeDeviceObjHasCap to be static to virnodedeviceobj
Signed-off-by: John Ferlan
---
src/conf/virnodedeviceobj.c | 36 +++-
src/conf/virnodedeviceobj.h | 12
src/libvirt_private.syms | 2 +-
src/node_device/node_device_driver.c | 34 ++
src/test/test_driver.c | 27 ++-
5 files changed, 60 insertions(+), 51 deletions(-)
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index f0c20c2..5a8dfe7 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -33,7 +33,7 @@
VIR_LOG_INIT("conf.virnodedeviceobj");
-int
+static int
virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
const char *cap)
{
@@ -496,6 +496,40 @@ virNodeDeviceObjNumOfDevices(virNodeDeviceObjListPtr devs,
}
+int
+virNodeDeviceObjGetNames(virNodeDeviceObjListPtr devs,
+ virConnectPtr conn,
+ virNodeDeviceObjListFilter aclfilter,
+ const char *cap,
+ char **const names,
+ int maxnames)
+{
+int nnames = 0;
+size_t i;
+
+for (i = 0; i < devs->count && nnames < maxnames; i++) {
+virNodeDeviceObjPtr obj = devs->objs[i];
+virNodeDeviceObjLock(obj);
+if (aclfilter && aclfilter(conn, obj->def) &&
+(!cap || virNodeDeviceObjHasCap(obj, cap))) {
+if (VIR_STRDUP(names[nnames++], obj->def->name) < 0) {
+virNodeDeviceObjUnlock(obj);
+goto failure;
+}
+}
+virNodeDeviceObjUnlock(obj);
+}
+
+return nnames;
+
+ failure:
+while (--nnames >= 0)
+VIR_FREE(names[nnames]);
+memset(names, 0, maxnames * sizeof(*names));
+return -1;
+}
+
+
#define MATCH(FLAG) ((flags & (VIR_CONNECT_LIST_NODE_DEVICES_CAP_ ## FLAG)) &&
\
virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_ ## FLAG))
static bool
diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 7d303a0..0acd8ce 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -40,10 +40,6 @@ struct _virNodeDeviceDriverState {
};
-int
-virNodeDeviceObjHasCap(const virNodeDeviceObj *dev,
- const char *cap);
-
virNodeDeviceObjPtr
virNodeDeviceObjFindByName(virNodeDeviceObjListPtr devs,
const char *name);
@@ -89,6 +85,14 @@ virNodeDeviceObjNumOfDevices(virNodeDeviceObjListPtr devs,
virNodeDeviceObjListFilter aclfilter);
int
+virNodeDeviceObjGetNames(virNodeDeviceObjListPtr devs,
+ virConnectPtr conn,
+ virNodeDeviceObjListFilter aclfilter,
+ const char *cap,
+ char **const names,
+ int maxnames);
+
+int
virNodeDeviceObjListExport(virConnectPtr conn,
virNodeDeviceObjList devobjs,
virNodeDevicePtr **devices,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 104ed88..5df6a5d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -946,8 +946,8 @@ virInterfaceObjUnlock;
virNodeDeviceObjAssignDef;
virNodeDeviceObjFindByName;
virNodeDeviceObjFindBySysfsPath;
+virNodeDeviceObjGetNames;
virNodeDeviceObjGetParentHost;
-virNodeDeviceObjHasCap;
virNodeDeviceObjListExport;
virNodeDeviceObjListFree;
virNodeDeviceObjLock;
diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
index f90b168..e850a22 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -174,14 +174,17 @@ nodeNumOfDevices(virConnectPtr conn,
return ndevs;
}
+
int
nodeListDevices(virConnectPtr conn,
const char *cap,
-char **const names, int maxnames,
+char **const names,
+int maxnames,
unsigned int flags)
{
-int ndevs = 0;
-size_t i;
+int nnames;
+
+memset(names, 0, maxnames * sizeof(*names));
if (virNodeListDevicesEnsureACL(conn) < 0)
return -1;
@@ -189,29 +192,12 @@ nodeListDevices(virConnectPtr conn,
virCheckFlags(0, -1);
nodeDeviceLock();
-for (i = 0; i < driver->devs.count && ndevs < maxnames; i++) {
-virNodeDeviceObjPtr obj = driver->devs.objs[i];
-virNodeDeviceObjLock(obj);
-if (vir
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 5:51 PM, Daniel P. Berrange wrote: > On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: > > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > > This patch is based on Martin's cache branch. > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > cpus='6-11'/> > > > > > > > > > > > > > Either the XML is malformed, or the indentation is wrong. The indentation > suggests you want nested XML elements, but the parent element is an empty > tag, so you've actually got a flat namespace here. > I validate it online, no errors, the result is: http://www.xmlvalidation.com/index.php?id=1&L=0&target=/xmlvalidation/start.jsp;jsessionid=5B8746DEE269CE0ADFB1B8F88CF5E94F > > > > > > > > > > Were we exposing the number of CLoS IDs before? Was there a discussion > > about it? Do we want to expose them? Probably yes, I'm just wondering. > > > > > What are CLoS IDs and what are they used for ? > > > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > > -- > libvir-list mailing list > libvir-list@redhat.com (mailto:libvir-list@redhat.com) > https://www.redhat.com/mailman/listinfo/libvir-list > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Split out -Wframe-larger-than warning from WARN_CLFAGS
On Wed, 2017-04-05 at 15:04 +0200, Ján Tomko wrote: > Introduce STRICT_FRAME_LIMIT_CFLAGS that will be used for > the library code and RELAXED_FRAME_LIMIT_CFLAGS for daemon code > and the test code. > > Raising the limit for tests allows building them with clang > with optimizations disabled. > --- > v2: > * use the strict limit for tools and daemon too > * append the limit to WARN_CFLAGS once per Makefile ACK if you fix the commit message as Roman suggested. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virISCSIGetSession: Don't leak memory
>> >> So what's the output from a "iscsiadm --mode session" on Michal's >> machine? And then if there is a duplicate - how did it happen? > > # iscsiadm --mode session > tcp: [31] 10.34.126.253:3260,1 iqn.2017-03.com.mprivozn:server > tcp: [32] virt-iscsi-server.usersys.redhat.com:3260,1 > iqn.2017-03.com.mprivozn:server > > where the URL from "line" [32] resolves to the IP from "line" [31]. > Ah - I see - so some sort of intentional (mis)configuration to avoid the text comparison because the IP address isn't being resolved... One would think that would/could be an iscsi(d) type bug. Does 'iscsiadm -m session -P 3' consider them different? Can you login to each? iscsiadm -m node -T $IQN -p $PORTAL --login where $PORTAL would be in your case 10.34.126.253:3260 or virt-iscsi-server.usersys.redhat.com:3260. What about: iscsiadm -m discovery -t sendtargets -p $PORTAL -P1 or Wonder how other tools react... "lsscsi -t -g" or "iscsi-ls ...". Do they see "two" devices which in the end are the same thing? That's kind of (well) scary. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 5:49 PM, Martin Kletzander wrote:
> On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote:
> > This patch is based on Martin's cache branch.
> >
> > This patch amends the cache bank capability as follow:
> >
> >
> >
> >
> >
> >
>
>
> Were we exposing the number of CLoS IDs before? Was there a discussion
> about it? Do we want to expose them? Probably yes, I'm just wondering.
>
Just saw Daniel’s comments, we may need it, with ‘nallocations’, is that okay?
Do you want me to do a reversion now?
>
> > Along with vircaps2xmltest case updated.
> >
> > Signed-off-by: Eli Qiao > (mailto:liyong.q...@intel.com)>
> > ---
> > src/conf/capabilities.c | 112 +--
> > src/conf/capabilities.h | 13 ++-
> > tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +
> > tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 2 +
> > tests/vircaps2xmltest.c | 11 +++
> > 5 files changed, 132 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index 416dd1a..75c0bec 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -52,6 +52,7 @@
> > #define VIR_FROM_THIS VIR_FROM_CAPABILITIES
> >
> > #define SYSFS_SYSTEM_PATH "/sys/devices/system/"
> > +#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
> >
> > VIR_LOG_INIT("conf.capabilities")
> >
> > @@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > virCapsHostCacheBankPtr *caches)
> > {
> > size_t i = 0;
> > + size_t j = 0;
> >
> > if (!ncaches)
> > return 0;
> > @@ -900,6 +902,18 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
> > bank->size >> (kilos * 10),
> > kilos ? "KiB" : "B",
> > cpus_str);
> > + virBufferAdjustIndent(buf, 2);
> > + for (j = 0; j < bank->ncontrol; j++) {
> > + bool min_kilos = !(bank->controls[j]->min % 1024);
> > + virBufferAsprintf(buf,
> > + " > + "type='%s' nclos='%u'/>\n",
> > + bank->controls[j]->min >> (min_kilos * 10),
> > + min_kilos ? "KiB" : "B",
> > + virCacheTypeToString(bank->controls[j]->type),
> > + bank->controls[j]->nclos);
> > + }
> > + virBufferAdjustIndent(buf, -2);
> >
> > VIR_FREE(cpus_str);
> > }
> > @@ -1516,10 +1530,93 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr
> > ptr)
> > if (!ptr)
> > return;
> >
> > + size_t i;
>
> Upstream frowns upon C99 initialization in the middle of the code.
>
Okay, I can more it before return.
>
> > virBitmapFree(ptr->cpus);
> > + for (i = 0; i < ptr->ncontrol; i++)
> > + VIR_FREE(ptr->controls[i]);
> > + VIR_FREE(ptr->controls);
> > VIR_FREE(ptr);
> > }
> >
> > +/* test which kinds of cache control supported
> > + * -1: don't support
> > + * 0: cat
> > + * 1: cdp
> > + */
> > +static int
> > +virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
> > +{
> > + int ret = -1;
> > + char *path = NULL;
> > + if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
> > + return -1;
> > +
> > + if (virFileExists(path)) {
> > + ret = 0;
> > + } else {
> > + VIR_FREE(path);
> > + if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level) <
> > 0)
> > + return -1;
> > + if (virFileExists(path))
> > + ret = 1;
> > + }
> > +
> > + VIR_FREE(path);
> > + return ret;
> > +}
> > +
> > +static int
> > +virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char*
> > type)
> > +{
> > + int ret = -1;
> > + char *path = NULL;
> > + char *cbm_mask = NULL;
> > + virCapsHostCacheControlPtr control;
> > +
> > + if (VIR_ALLOC(control) < 0)
> > + goto cleanup;
> > +
> > + if (virFileReadValueUint(&control->nclos,
> > + SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
> > + bank->level,
> > + type) < 0)
> > + goto cleanup;
> > +
> > + if (virFileReadValueString(&cbm_mask,
> > + SYSFS_RESCTRL_PATH
> > + "info/L%u%s/cbm_mask",
> > + bank->level,
> > + type) < 0)
> > + goto cleanup;
> > +
> > + virStringTrimOptionalNewline(cbm_mask);
> > +
> > + control->min = bank->size / (strlen(cbm_mask) * 4);
> > +
> > + if (STREQ("", type))
> > + control->type = VIR_CACHE_TYPE_UNIFIED;
> > + else if (STREQ("CODE", type))
> > + control->type = VIR_CACHE_TYPE_INSTRUCTION;
> > + else if (STREQ("DATA", type))
> > + control->type = VIR_CACHE_TYPE_DATA;
> > + else
> > + goto cleanup;
> > +
> > + if (VIR_APPEND_ELEMENT(bank->controls,
> > + bank->ncontrol,
> > + control) < 0)
> > + goto error;
> > +
> > + ret = 0;
> > +
> > + cleanup:
> > + VIR_FREE(path);
> > + return ret;
> > + error:
> > + VIR_FREE(control);
> > + return -1;
> >
>
>
> The return value is not used anywhere.
yes, I think we can ignore this value or not? if we enabled resctrl (we can
always read correct thing from /sys/fs/resctrl/info/…)
>
> > +}
> > +
> > int
> > virCapabilitiesInitCaches(virCapsPtr caps)
> > {
> > @@ -1533,11 +1630,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
> > struct dirent *ent = NULL;
> > virCapsHostCacheBankPtr bank = NULL;
> >
> > - /* Minimum level to expose in capabilities. Can be lowered or removed
>
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 06:29:47PM +0800, Eli Qiao wrote: > > > On Thursday, 6 April 2017 at 6:10 PM, Daniel P. Berrange wrote: > > > On Thu, Apr 06, 2017 at 12:05:54PM +0200, Martin Kletzander wrote: > > > On Thu, Apr 06, 2017 at 10:51:44AM +0100, Daniel P. Berrange wrote: > > > > On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: > > > > > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > > > > > This patch is based on Martin's cache branch. > > > > > > > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > > > > cpus='0-5'/> > > > > > > > > > > > > > > > > > cpus='6-11'/> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Either the XML is malformed, or the indentation is wrong. The > > > > indentation > > > > suggests you want nested XML elements, but the parent element is an > > > > empty > > > > tag, so you've actually got a flat namespace here. > > > > > > > > > > > > > > Were we exposing the number of CLoS IDs before? Was there a discussion > > > > > about it? Do we want to expose them? Probably yes, I'm just wondering. > > > > > > > > > > > > > > > > > What are CLoS IDs and what are they used for ? > > > > > > Effectively an ID for the allocation. The hardware has a limited number > > > of them, in this case 4. I can't remember whether that number is > > > per-bank, but it would not make much sense otherwise. > > > > > > > > > So, if guests are requesting a private cache allocation, and cos id == 4, > > then we can only run 4 guests ? > > > > > > > I think yes, but it’s per bank resource (the bank here is equal to cache > id), if you have 2 banks , you can create 8 guests (each has 1 bank > allocation) Ok, well if it is resource limiting in this way, then I think it is important to expose in the capabilities. This information would be needed by openstack schedular in order to avoid placing too guests on the same host when they have requested cache allocation. Picking a more obvious attr name would be nice though eg 'nallocations' Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 5:49 PM, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > This patch is based on Martin's cache branch. > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > > Were we exposing the number of CLoS IDs before? Was there a discussion > about it? Do we want to expose them? Probably yes, I'm just wondering. > > > We don’t expose it in previous discussion as in my old design, it is saved in a global internal struct, and we will use it to limit the cache allocation number. For now as you expose all these to capabilities XML, I want expose it out which can be reference by resctrl utils later. Thought? > > -- > > libvir-list mailing list > > libvir-list@redhat.com (mailto:libvir-list@redhat.com) > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > > -- > libvir-list mailing list > libvir-list@redhat.com (mailto:libvir-list@redhat.com) > https://www.redhat.com/mailman/listinfo/libvir-list > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thursday, 6 April 2017 at 6:10 PM, Daniel P. Berrange wrote: > On Thu, Apr 06, 2017 at 12:05:54PM +0200, Martin Kletzander wrote: > > On Thu, Apr 06, 2017 at 10:51:44AM +0100, Daniel P. Berrange wrote: > > > On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: > > > > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > > > > This patch is based on Martin's cache branch. > > > > > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > > > > cpus='0-5'/> > > > > > > > > > > > > > > cpus='6-11'/> > > > > > > > > > > > > > > > > > > > > > > > > > > > Either the XML is malformed, or the indentation is wrong. The indentation > > > suggests you want nested XML elements, but the parent element is an empty > > > tag, so you've actually got a flat namespace here. > > > > > > > > > > > Were we exposing the number of CLoS IDs before? Was there a discussion > > > > about it? Do we want to expose them? Probably yes, I'm just wondering. > > > > > > > > > > > > > What are CLoS IDs and what are they used for ? > > > > Effectively an ID for the allocation. The hardware has a limited number > > of them, in this case 4. I can't remember whether that number is > > per-bank, but it would not make much sense otherwise. > > > > > So, if guests are requesting a private cache allocation, and cos id == 4, > then we can only run 4 guests ? > > I think yes, but it’s per bank resource (the bank here is equal to cache id), if you have 2 banks , you can create 8 guests (each has 1 bank allocation) As far as I know, the number of clos id is 16 on most of Intel xeon CPUs > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > > -- > libvir-list mailing list > libvir-list@redhat.com (mailto:libvir-list@redhat.com) > https://www.redhat.com/mailman/listinfo/libvir-list > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] leasetime support per and
Sorry, we missed this 2nd patch posting. FYI, when sending updated versions of patches, we generally recommend to send them as a new top level thread, with 'PATCH v2' in the subject line, not as reply to the v1 patch. We find that makes it less likely to miss the second version when processing email. > From d48f957ab78310d864b356b4a25b9a29722ca736 Mon Sep 17 00:00:00 2001 > From: Alberto Ruiz > Date: Thu, 6 Oct 2016 21:29:40 +0100 > Subject: [PATCH] leasetime support per and > > Support for custom dhcp wide and per host leasetime. > > It is specified as a child tag for : > > 24h > ... > > > And as an attribute for : > > > > > These are the different notations: > > -1 (infinite/unlimited lease) > 120 (seconds are the default unit, 120 seconds is the minimum, if less is > specified it will use 120) > 300s (seconds) > 5m (minutes) > 24h (hours) > 7d (days) What does '0' mean - i guess we're considering '0' to mean the default dnsmasq lease time for sake of upgrade from previous versions ? Using this syntax means that applications processing XML need to first extract the attribute, and then parse it to separate the value from the unit. In most other places, we use separate attribute to denote the units. So I think we'd probably be better doing 24 ... and Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 12:05:54PM +0200, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 10:51:44AM +0100, Daniel P. Berrange wrote: > > On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: > > > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > > > This patch is based on Martin's cache branch. > > > > > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > > cpus='0-5'/> > > > > > > > > > > > cpus='6-11'/> > > > > > > > > Either the XML is malformed, or the indentation is wrong. The indentation > > suggests you want nested XML elements, but the parent element is an empty > > tag, so you've actually got a flat namespace here. > > > > > > > > > > > > Were we exposing the number of CLoS IDs before? Was there a discussion > > > about it? Do we want to expose them? Probably yes, I'm just wondering. > > > > What are CLoS IDs and what are they used for ? > > > > Effectively an ID for the allocation. The hardware has a limited number > of them, in this case 4. I can't remember whether that number is > per-bank, but it would not make much sense otherwise. So, if guests are requesting a private cache allocation, and cos id == 4, then we can only run 4 guests ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 10:51:44AM +0100, Daniel P. Berrange wrote: On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > This patch is based on Martin's cache branch. > > This patch amends the cache bank capability as follow: > > > > > Either the XML is malformed, or the indentation is wrong. The indentation suggests you want nested XML elements, but the parent element is an empty tag, so you've actually got a flat namespace here. > Were we exposing the number of CLoS IDs before? Was there a discussion about it? Do we want to expose them? Probably yes, I'm just wondering. What are CLoS IDs and what are they used for ? Effectively an ID for the allocation. The hardware has a limited number of them, in this case 4. I can't remember whether that number is per-bank, but it would not make much sense otherwise. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- 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 v2 01/12] conf: Fix possible memleak in capabilities
On Thu, Apr 06, 2017 at 11:52:54AM +0200, Martin Kletzander wrote:
> On Thu, Apr 06, 2017 at 11:29:21AM +0200, Erik Skultety wrote:
> > On Thu, Apr 06, 2017 at 11:12:54AM +0200, Martin Kletzander wrote:
> > > On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote:
> > > > On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote:
> > > > > If formatting NUMA topology fails, the function returns immediatelly,
> > > > > but the buffer structure allocated on the stack references lot of
> > > > > heap-allocated memory and that would get lost in such case.
> > > > >
> > > > > Signed-off-by: Martin Kletzander
> > > > > ---
> > > > > src/conf/capabilities.c | 6 +-
> > > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > > > > index 08907aced1b9..be95c50cfb67 100644
> > > > > --- a/src/conf/capabilities.c
> > > > > +++ b/src/conf/capabilities.c
> > > > > @@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > > > > if (caps->host.nnumaCell &&
> > > > > virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
> > > > >caps->host.numaCell) < 0)
> > > > > -return NULL;
> > > > > +goto error;
> > > >
> > > > Personally, I'd more like cleanup, but looking at other XML formatting
> > > > methods,
> > > > I'm fine with this as well, at least we stay consistent.
> > > >
> > > > >
> > > > > for (i = 0; i < caps->host.nsecModels; i++) {
> > > > > virBufferAddLit(&buf, "\n");
> > > > > @@ -1072,6 +1072,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > > > > return NULL;
> > > > >
> > > >
> > > > Git discarded the context here, so squash this in:
> > > >
> > > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > > > index be95c50cf..ea6d4b19d 100644
> > > > --- a/src/conf/capabilities.c
> > > > +++ b/src/conf/capabilities.c
> > > > @@ -1069,7 +1069,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > > > virBufferAddLit(&buf, "\n");
> > > >
> > > > if (virBufferCheckError(&buf) < 0)
> > > > -return NULL;
> > > > +goto error;
> > > >
> > > > return virBufferContentAndReset(&buf);
> > > >
> > >
> > > I don't really understand why would I need to do that.
> >
> > Because buf->content might be non-NULL.
> >
>
> Buffers are automatically reset on errors as there is no need for the
> data. It also makes the function epilogues and error handling easier
> and shorter (e.g. this case). See virBufferSetError() for more info.
Fair enough, I missed the VIR_FREE at the top of the function when I was looking
at it.
>
> > >
> > > >
> > > > > return virBufferContentAndReset(&buf);
> > > > > +
> > > > > + error:
> > > > > +virBufferFreeAndReset(&buf);
> > > > > +return NULL;
> > > > > }
> > > > >
> > > > > /* get the maximum ID of cpus in the host */
> > > > > --
> > > > > 2.12.2
> > > > >
> > > >
> > > > ACK with the bit above squashed in.
> > > >
> > > > Erik
> >
> >
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Move some functions to qemu_capspriv.h
On Thu, Apr 06, 2017 at 11:33:03AM +0200, Andrea Bolognani wrote: On Thu, 2017-04-06 at 10:00 +0200, Martin Kletzander wrote: > @@ -8,6 +8,10 @@ > # include > > # include "qemu/qemu_capabilities.h" > +# define __QEMU_CAPSPRIV_H_ALLOW__ > +# include "qemu/qemu_capspriv.h" > +# undef __QEMU_CAPSPRIV_H_ALLOW__ > + We should, ideally, make some global PRIV_ENABLE macro that would be set in Makefile for all tests. And all private headers would check that macro. We wouldn't need to add these in the tests, and it might actually be a bit more future-proof. I don't necessarily disagree, but I think we have a long way ahead before something like that can be implemented. There are a lot of functions that are exposed to everyone just because we want to test them, and I don't think all modules have a corresponding private header file. So I'd work towards that goal first. I didn't mean to have like a central location for all of them. Just have one macro. Anyway I missed the fact that we need to include them in the corresponding .c files and that wouldn't work. So act like I didn't reply. The idea in my head sounded really cool, though. -- Andrea Bolognani / Red Hat / Virtualization -- 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 v2 01/12] conf: Fix possible memleak in capabilities
On Thu, Apr 06, 2017 at 11:29:21AM +0200, Erik Skultety wrote:
On Thu, Apr 06, 2017 at 11:12:54AM +0200, Martin Kletzander wrote:
On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote:
> On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote:
> > If formatting NUMA topology fails, the function returns immediatelly,
> > but the buffer structure allocated on the stack references lot of
> > heap-allocated memory and that would get lost in such case.
> >
> > Signed-off-by: Martin Kletzander
> > ---
> > src/conf/capabilities.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index 08907aced1b9..be95c50cfb67 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > if (caps->host.nnumaCell &&
> > virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
> >caps->host.numaCell) < 0)
> > -return NULL;
> > +goto error;
>
> Personally, I'd more like cleanup, but looking at other XML formatting
methods,
> I'm fine with this as well, at least we stay consistent.
>
> >
> > for (i = 0; i < caps->host.nsecModels; i++) {
> > virBufferAddLit(&buf, "\n");
> > @@ -1072,6 +1072,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > return NULL;
> >
>
> Git discarded the context here, so squash this in:
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index be95c50cf..ea6d4b19d 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -1069,7 +1069,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> virBufferAddLit(&buf, "\n");
>
> if (virBufferCheckError(&buf) < 0)
> -return NULL;
> +goto error;
>
> return virBufferContentAndReset(&buf);
>
I don't really understand why would I need to do that.
Because buf->content might be non-NULL.
Buffers are automatically reset on errors as there is no need for the
data. It also makes the function epilogues and error handling easier
and shorter (e.g. this case). See virBufferSetError() for more info.
>
> > return virBufferContentAndReset(&buf);
> > +
> > + error:
> > +virBufferFreeAndReset(&buf);
> > +return NULL;
> > }
> >
> > /* get the maximum ID of cpus in the host */
> > --
> > 2.12.2
> >
>
> ACK with the bit above squashed in.
>
> Erik
--
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] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 11:49:14AM +0200, Martin Kletzander wrote: > On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote: > > This patch is based on Martin's cache branch. > > > > This patch amends the cache bank capability as follow: > > > > > > > > > > Either the XML is malformed, or the indentation is wrong. The indentation suggests you want nested XML elements, but the parent element is an empty tag, so you've actually got a flat namespace here. > > > > Were we exposing the number of CLoS IDs before? Was there a discussion > about it? Do we want to expose them? Probably yes, I'm just wondering. What are CLoS IDs and what are they used for ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Expose resource control capabilites on cache bank
On Thu, Apr 06, 2017 at 04:20:06PM +0800, Eli Qiao wrote:
This patch is based on Martin's cache branch.
This patch amends the cache bank capability as follow:
Were we exposing the number of CLoS IDs before? Was there a discussion
about it? Do we want to expose them? Probably yes, I'm just wondering.
Along with vircaps2xmltest case updated.
Signed-off-by: Eli Qiao
---
src/conf/capabilities.c | 112 +--
src/conf/capabilities.h | 13 ++-
tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 2 +
tests/vircaps2xmltest.c | 11 +++
5 files changed, 132 insertions(+), 8 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 416dd1a..75c0bec 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -52,6 +52,7 @@
#define VIR_FROM_THIS VIR_FROM_CAPABILITIES
#define SYSFS_SYSTEM_PATH "/sys/devices/system/"
+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
VIR_LOG_INIT("conf.capabilities")
@@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
virCapsHostCacheBankPtr *caches)
{
size_t i = 0;
+size_t j = 0;
if (!ncaches)
return 0;
@@ -900,6 +902,18 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
bank->size >> (kilos * 10),
kilos ? "KiB" : "B",
cpus_str);
+virBufferAdjustIndent(buf, 2);
+for (j = 0; j < bank->ncontrol; j++) {
+bool min_kilos = !(bank->controls[j]->min % 1024);
+virBufferAsprintf(buf,
+ "\n",
+ bank->controls[j]->min >> (min_kilos * 10),
+ min_kilos ? "KiB" : "B",
+ virCacheTypeToString(bank->controls[j]->type),
+ bank->controls[j]->nclos);
+}
+virBufferAdjustIndent(buf, -2);
VIR_FREE(cpus_str);
}
@@ -1516,10 +1530,93 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
if (!ptr)
return;
+size_t i;
Upstream frowns upon C99 initialization in the middle of the code.
virBitmapFree(ptr->cpus);
+for (i = 0; i < ptr->ncontrol; i++)
+VIR_FREE(ptr->controls[i]);
+VIR_FREE(ptr->controls);
VIR_FREE(ptr);
}
+/* test which kinds of cache control supported
+ * -1: don't support
+ * 0: cat
+ * 1: cdp
+ */
+static int
+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
+{
+int ret = -1;
+char *path = NULL;
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
+return -1;
+
+if (virFileExists(path)) {
+ret = 0;
+} else {
+VIR_FREE(path);
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level)
< 0)
+return -1;
+if (virFileExists(path))
+ret = 1;
+}
+
+VIR_FREE(path);
+return ret;
+}
+
+static int
+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char* type)
+{
+int ret = -1;
+char *path = NULL;
+char *cbm_mask = NULL;
+virCapsHostCacheControlPtr control;
+
+if (VIR_ALLOC(control) < 0)
+goto cleanup;
+
+if (virFileReadValueUint(&control->nclos,
+ SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+if (virFileReadValueString(&cbm_mask,
+ SYSFS_RESCTRL_PATH
+ "info/L%u%s/cbm_mask",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+virStringTrimOptionalNewline(cbm_mask);
+
+control->min = bank->size / (strlen(cbm_mask) * 4);
+
+if (STREQ("", type))
+control->type = VIR_CACHE_TYPE_UNIFIED;
+else if (STREQ("CODE", type))
+control->type = VIR_CACHE_TYPE_INSTRUCTION;
+else if (STREQ("DATA", type))
+control->type = VIR_CACHE_TYPE_DATA;
+else
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(bank->controls,
+ bank->ncontrol,
+ control) < 0)
+goto error;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+return ret;
+ error:
+VIR_FREE(control);
+return -1;
The return value is not used anywhere.
+}
+
int
virCapabilitiesInitCaches(virCapsPtr caps)
{
@@ -1533,11 +1630,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
struct dirent *ent = NULL;
virCapsHostCacheBankPtr bank = NULL;
-/* Minimum level to expose in capabilities. Can be lowered or removed
(with
- * the appropriate code below), but should not be increased, because we'd
- * lose information. */
-const int cache_min_level = 3;
-
You are removing this and only reporting it for
Re: [libvirt] [PATCH] qemu: Move some functions to qemu_capspriv.h
On Wed, Apr 05, 2017 at 04:41:57PM +0200, Andrea Bolognani wrote: This header file has been created so that we can expose internal functions to the test suite without making them public: those in qemu_capabilities.h bearing the comment /* Only for use by test suite */ are obvious candidates for being moved over. --- src/qemu/qemu_capabilities.h | 19 --- src/qemu/qemu_capspriv.h | 22 ++ tests/qemuhelptest.c | 4 3 files changed, 26 insertions(+), 19 deletions(-) ACK Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Move some functions to qemu_capspriv.h
On Thu, 2017-04-06 at 10:00 +0200, Martin Kletzander wrote: > > @@ -8,6 +8,10 @@ > > # include > > > > # include "qemu/qemu_capabilities.h" > > +# define __QEMU_CAPSPRIV_H_ALLOW__ > > +# include "qemu/qemu_capspriv.h" > > +# undef __QEMU_CAPSPRIV_H_ALLOW__ > > + > > We should, ideally, make some global PRIV_ENABLE macro that would be set > in Makefile for all tests. And all private headers would check that > macro. We wouldn't need to add these in the tests, and it might > actually be a bit more future-proof. I don't necessarily disagree, but I think we have a long way ahead before something like that can be implemented. There are a lot of functions that are exposed to everyone just because we want to test them, and I don't think all modules have a corresponding private header file. So I'd work towards that goal first. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities
On Thu, Apr 06, 2017 at 11:12:54AM +0200, Martin Kletzander wrote:
> On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote:
> > On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote:
> > > If formatting NUMA topology fails, the function returns immediatelly,
> > > but the buffer structure allocated on the stack references lot of
> > > heap-allocated memory and that would get lost in such case.
> > >
> > > Signed-off-by: Martin Kletzander
> > > ---
> > > src/conf/capabilities.c | 6 +-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > > index 08907aced1b9..be95c50cfb67 100644
> > > --- a/src/conf/capabilities.c
> > > +++ b/src/conf/capabilities.c
> > > @@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > > if (caps->host.nnumaCell &&
> > > virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
> > >caps->host.numaCell) < 0)
> > > -return NULL;
> > > +goto error;
> >
> > Personally, I'd more like cleanup, but looking at other XML formatting
> > methods,
> > I'm fine with this as well, at least we stay consistent.
> >
> > >
> > > for (i = 0; i < caps->host.nsecModels; i++) {
> > > virBufferAddLit(&buf, "\n");
> > > @@ -1072,6 +1072,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > > return NULL;
> > >
> >
> > Git discarded the context here, so squash this in:
> >
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index be95c50cf..ea6d4b19d 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -1069,7 +1069,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > virBufferAddLit(&buf, "\n");
> >
> > if (virBufferCheckError(&buf) < 0)
> > -return NULL;
> > +goto error;
> >
> > return virBufferContentAndReset(&buf);
> >
>
> I don't really understand why would I need to do that.
Because buf->content might be non-NULL.
>
> >
> > > return virBufferContentAndReset(&buf);
> > > +
> > > + error:
> > > +virBufferFreeAndReset(&buf);
> > > +return NULL;
> > > }
> > >
> > > /* get the maximum ID of cpus in the host */
> > > --
> > > 2.12.2
> > >
> >
> > ACK with the bit above squashed in.
> >
> > Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/12] Add host cache information into capabilities
On Thu, Apr 06, 2017 at 02:04:04PM +0800, Eli Qiao wrote:
+
+VIR_ENUM_IMPL(virCache, VIR_CACHE_TYPE_LAST,
+ "unified",
+ "instruction",
+ "data")
+
};
+typedef enum {
+ VIR_CACHE_TYPE_DATA,
+ VIR_CACHE_TYPE_INSTRUCTION,
+ VIR_CACHE_TYPE_UNIFIED,
+
+ VIR_CACHE_TYPE_LAST
+} virCacheType;
+
The sequence is wrong, it should be :
VIR_CACHE_TYPE_UNIFIED,
VIR_CACHE_TYPE_INSTRUCTION,
VIR_CACHE_TYPE_DATA,
Hehe, good catch. Since the internal values are not exposed anywhere,
the test data aren't affected. I wish I remembered why I changed that.
For future reviewers, consider the following squashed in:
diff --git i/src/conf/capabilities.h w/src/conf/capabilities.h
index e099cccf6d39..60c8218ce3e5 100644
--- i/src/conf/capabilities.h
+++ w/src/conf/capabilities.h
@@ -139,9 +139,9 @@ struct _virCapsHostSecModel {
};
typedef enum {
-VIR_CACHE_TYPE_DATA,
-VIR_CACHE_TYPE_INSTRUCTION,
VIR_CACHE_TYPE_UNIFIED,
+VIR_CACHE_TYPE_INSTRUCTION,
+VIR_CACHE_TYPE_DATA,
VIR_CACHE_TYPE_LAST
} virCacheType;
--
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities
On Thu, Apr 06, 2017 at 09:50:49AM +0200, Erik Skultety wrote:
On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote:
If formatting NUMA topology fails, the function returns immediatelly,
but the buffer structure allocated on the stack references lot of
heap-allocated memory and that would get lost in such case.
Signed-off-by: Martin Kletzander
---
src/conf/capabilities.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 08907aced1b9..be95c50cfb67 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
if (caps->host.nnumaCell &&
virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
caps->host.numaCell) < 0)
-return NULL;
+goto error;
Personally, I'd more like cleanup, but looking at other XML formatting methods,
I'm fine with this as well, at least we stay consistent.
for (i = 0; i < caps->host.nsecModels; i++) {
virBufferAddLit(&buf, "\n");
@@ -1072,6 +1072,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
return NULL;
Git discarded the context here, so squash this in:
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index be95c50cf..ea6d4b19d 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1069,7 +1069,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&buf, "\n");
if (virBufferCheckError(&buf) < 0)
-return NULL;
+goto error;
return virBufferContentAndReset(&buf);
I don't really understand why would I need to do that.
return virBufferContentAndReset(&buf);
+
+ error:
+virBufferFreeAndReset(&buf);
+return NULL;
}
/* get the maximum ID of cpus in the host */
--
2.12.2
ACK with the bit above squashed in.
Erik
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 12/12] tests: Add resctrl test for vircaps2xmltest
On Thu, Apr 06, 2017 at 09:38:35AM +0800, Eli Qiao wrote: On Wednesday, 5 April 2017 at 10:36 PM, Martin Kletzander wrote: Add info from yet another machine, this time with resctrl data so that we can extend tests easily in a test-driven way. Signed-off-by: Martin Kletzander mailto:mklet...@redhat.com)> --- tests/vircaps2xmldata/linux-resctrl/resctrl/cpus | 1 + .../linux-resctrl/resctrl/info/L3/cbm_mask | 1 + .../linux-resctrl/resctrl/info/L3/min_cbm_bits | 1 + .../linux-resctrl/resctrl/info/L3/num_closids | 1 + .../linux-resctrl/resctrl/manualres/cpus | 1 + .../linux-resctrl/resctrl/manualres/schemata | 1 + .../linux-resctrl/resctrl/manualres/tasks | 0 .../vircaps2xmldata/linux-resctrl/resctrl/schemata | 1 + tests/vircaps2xmldata/linux-resctrl/resctrl/tasks | 0 This is great and it is what we want before enable CAT in libvirt, but seems that if we want to test CDP enabled, we need another linux-resctrl directory? how about separate resctrl from linux-resctrl? Sure, I hope there will be many of them, each testing different scenarios. linux-resctrl2, linux-resctrlN, linux-cdp-resctrl, linux-whatever-new-directory, whenever we want any new one. Eli. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Expose resource control capabilites on cache bank
This patch is based on Martin's cache branch.
This patch amends the cache bank capability as follow:
Along with vircaps2xmltest case updated.
Signed-off-by: Eli Qiao
---
src/conf/capabilities.c | 112 +--
src/conf/capabilities.h | 13 ++-
tests/vircaps2xmldata/vircaps-x86_64-caches.xml | 2 +
tests/vircaps2xmldata/vircaps-x86_64-resctrl.xml | 2 +
tests/vircaps2xmltest.c | 11 +++
5 files changed, 132 insertions(+), 8 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 416dd1a..75c0bec 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -52,6 +52,7 @@
#define VIR_FROM_THIS VIR_FROM_CAPABILITIES
#define SYSFS_SYSTEM_PATH "/sys/devices/system/"
+#define SYSFS_RESCTRL_PATH "/sys/fs/resctrl/"
VIR_LOG_INIT("conf.capabilities")
@@ -873,6 +874,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
virCapsHostCacheBankPtr *caches)
{
size_t i = 0;
+size_t j = 0;
if (!ncaches)
return 0;
@@ -900,6 +902,18 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
bank->size >> (kilos * 10),
kilos ? "KiB" : "B",
cpus_str);
+virBufferAdjustIndent(buf, 2);
+for (j = 0; j < bank->ncontrol; j++) {
+bool min_kilos = !(bank->controls[j]->min % 1024);
+virBufferAsprintf(buf,
+ "\n",
+ bank->controls[j]->min >> (min_kilos * 10),
+ min_kilos ? "KiB" : "B",
+ virCacheTypeToString(bank->controls[j]->type),
+ bank->controls[j]->nclos);
+}
+virBufferAdjustIndent(buf, -2);
VIR_FREE(cpus_str);
}
@@ -1516,10 +1530,93 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
if (!ptr)
return;
+size_t i;
virBitmapFree(ptr->cpus);
+for (i = 0; i < ptr->ncontrol; i++)
+VIR_FREE(ptr->controls[i]);
+VIR_FREE(ptr->controls);
VIR_FREE(ptr);
}
+/* test which kinds of cache control supported
+ * -1: don't support
+ * 0: cat
+ * 1: cdp
+ */
+static int
+virCapabilitiesGetCacheControlType(virCapsHostCacheBankPtr bank)
+{
+int ret = -1;
+char *path = NULL;
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%u", bank->level) < 0)
+return -1;
+
+if (virFileExists(path)) {
+ret = 0;
+} else {
+VIR_FREE(path);
+if (virAsprintf(&path, SYSFS_RESCTRL_PATH "info/L%uCODE", bank->level)
< 0)
+return -1;
+if (virFileExists(path))
+ret = 1;
+}
+
+VIR_FREE(path);
+return ret;
+}
+
+static int
+virCapabilitiesGetCacheControl(virCapsHostCacheBankPtr bank, const char* type)
+{
+int ret = -1;
+char *path = NULL;
+char *cbm_mask = NULL;
+virCapsHostCacheControlPtr control;
+
+if (VIR_ALLOC(control) < 0)
+goto cleanup;
+
+if (virFileReadValueUint(&control->nclos,
+ SYSFS_RESCTRL_PATH "info/L%u%s/num_closids",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+if (virFileReadValueString(&cbm_mask,
+ SYSFS_RESCTRL_PATH
+ "info/L%u%s/cbm_mask",
+ bank->level,
+ type) < 0)
+goto cleanup;
+
+virStringTrimOptionalNewline(cbm_mask);
+
+control->min = bank->size / (strlen(cbm_mask) * 4);
+
+if (STREQ("", type))
+control->type = VIR_CACHE_TYPE_UNIFIED;
+else if (STREQ("CODE", type))
+control->type = VIR_CACHE_TYPE_INSTRUCTION;
+else if (STREQ("DATA", type))
+control->type = VIR_CACHE_TYPE_DATA;
+else
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(bank->controls,
+ bank->ncontrol,
+ control) < 0)
+goto error;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+return ret;
+ error:
+VIR_FREE(control);
+return -1;
+}
+
int
virCapabilitiesInitCaches(virCapsPtr caps)
{
@@ -1533,11 +1630,6 @@ virCapabilitiesInitCaches(virCapsPtr caps)
struct dirent *ent = NULL;
virCapsHostCacheBankPtr bank = NULL;
-/* Minimum level to expose in capabilities. Can be lowered or removed
(with
- * the appropriate code below), but should not be increased, because we'd
- * lose information. */
-const int cache_min_level = 3;
-
/* offline CPUs don't provide cache info */
if (virFileReadValueBitmap(&cpus, SYSFS_SYSTEM_PATH "cpu/online") < 0)
return -1;
@@ -1595,12 +1687,19 @@ virCapabilitiesInitCaches(virCapsPtr caps)
pos, ent->d_name) < 0)
goto cleanup;
-if (bank->lev
Re: [libvirt] [PATCH] qemu: Move some functions to qemu_capspriv.h
On Wed, Apr 05, 2017 at 04:41:57PM +0200, Andrea Bolognani wrote: This header file has been created so that we can expose internal functions to the test suite without making them public: those in qemu_capabilities.h bearing the comment /* Only for use by test suite */ are obvious candidates for being moved over. --- src/qemu/qemu_capabilities.h | 19 --- src/qemu/qemu_capspriv.h | 22 ++ tests/qemuhelptest.c | 4 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 24e2f38..cca9a12 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -473,11 +473,6 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps, void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, const char *machineType); -/* Only for use by test suite */ -void virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, - virGICCapability *capabilities, - size_t ncapabilities); - virQEMUCapsCachePtr virQEMUCapsCacheNew(const char *libDir, const char *cacheDir, uid_t uid, gid_t gid); @@ -499,20 +494,6 @@ int virQEMUCapsGetDefaultVersion(virCapsPtr caps, virQEMUCapsCachePtr capsCache, unsigned int *version); -/* Only for use by test suite */ -int virQEMUCapsParseHelpStr(const char *qemu, -const char *str, -virQEMUCapsPtr qemuCaps, -unsigned int *version, -bool *is_kvm, -unsigned int *kvm_version, -bool check_yajl, -const char *qmperr); -/* Only for use by test suite */ -int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); -/* Only for use by test suite */ -void virQEMUCapsInitQMPBasicArch(virQEMUCapsPtr qemuCaps); - VIR_ENUM_DECL(virQEMUCaps); bool virQEMUCapsSupportsChardev(const virDomainDef *def, diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 61ccd45..9818859 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -84,6 +84,9 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps, virCPUDefPtr cpu); void +virQEMUCapsInitQMPBasicArch(virQEMUCapsPtr qemuCaps); + +void virQEMUCapsSetCPUModelInfo(virQEMUCapsPtr qemuCaps, virDomainVirtType type, qemuMonitorCPUModelInfoPtr modelInfo); @@ -92,4 +95,23 @@ virCPUDefPtr virQEMUCapsProbeHostCPUForEmulator(virCapsPtr caps, virQEMUCapsPtr qemuCaps, virDomainVirtType type); + +void +virQEMUCapsSetGICCapabilities(virQEMUCapsPtr qemuCaps, + virGICCapability *capabilities, + size_t ncapabilities); + +int +virQEMUCapsParseHelpStr(const char *qemu, +const char *str, +virQEMUCapsPtr qemuCaps, +unsigned int *version, +bool *is_kvm, +unsigned int *kvm_version, +bool check_yajl, +const char *qmperr); + +int +virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, + const char *str); #endif diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index e3751b2..d80225c 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -8,6 +8,10 @@ # include # include "qemu/qemu_capabilities.h" +# define __QEMU_CAPSPRIV_H_ALLOW__ +# include "qemu/qemu_capspriv.h" +# undef __QEMU_CAPSPRIV_H_ALLOW__ + We should, ideally, make some global PRIV_ENABLE macro that would be set in Makefile for all tests. And all private headers would check that macro. We wouldn't need to add these in the tests, and it might actually be a bit more future-proof. # include "viralloc.h" # include "virstring.h" -- 2.7.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
Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities
On Thu, Apr 06, 2017 at 09:50:49 +0200, Erik Skultety wrote: > On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote: > > If formatting NUMA topology fails, the function returns immediatelly, > > but the buffer structure allocated on the stack references lot of > > heap-allocated memory and that would get lost in such case. > > > > Signed-off-by: Martin Kletzander > > --- > > src/conf/capabilities.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c > > index 08907aced1b9..be95c50cfb67 100644 > > --- a/src/conf/capabilities.c > > +++ b/src/conf/capabilities.c > > @@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps) > > if (caps->host.nnumaCell && > > virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell, > >caps->host.numaCell) < 0) > > -return NULL; > > +goto error; > > Personally, I'd more like cleanup, but looking at other XML formatting > methods, > I'm fine with this as well, at least we stay consistent. "cleanup" label should be used if the success path passes through the label as well. If only the error path passes through it it should be called "error". signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] storage: Avoid leak in virStorageUtilGlusterExtractPoolSources()
On Thu, Apr 06, 2017 at 09:53:11 +0200, Andrea Bolognani wrote: > The contents of volname would be leaked if the function were > to be passed an invalid pooltype by the caller. Which won't ever happen. > Make sure the memory is released instead. > --- ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/8] qemu: Don't overwrite existing error in qemuMigrationReset
https://bugzilla.redhat.com/show_bug.cgi?id=1439130
Signed-off-by: Jiri Denemark
---
src/qemu/qemu_migration.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1feb320b8..d8222fe3b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5871,15 +5871,22 @@ qemuMigrationReset(virQEMUDriverPtr driver,
qemuDomainAsyncJob job)
{
qemuMonitorMigrationCaps cap;
+virErrorPtr err = virSaveLastError();
if (!virDomainObjIsActive(vm))
-return;
+goto cleanup;
if (qemuMigrationResetTLS(driver, vm, job) < 0)
-return;
+goto cleanup;
for (cap = 0; cap < QEMU_MONITOR_MIGRATION_CAPS_LAST; cap++) {
if (qemuMigrationSetOption(driver, vm, cap, false, job) < 0)
-return;
+goto cleanup;
+}
+
+ cleanup:
+if (err) {
+virSetError(err);
+virFreeError(err);
}
}
--
2.12.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 02/12] util: Fix virDirRead() description
On Wed, Apr 05, 2017 at 04:36:25PM +0200, Martin Kletzander wrote: > Signed-off-by: Martin Kletzander > --- > src/util/virfile.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/util/virfile.c b/src/util/virfile.c > index a91c2c349501..c0f448d3437d 100644 > --- a/src/util/virfile.c > +++ b/src/util/virfile.c > @@ -2861,11 +2861,11 @@ virDirOpenQuiet(DIR **dirp, const char *name) > /** > * virDirRead: > * @dirp: directory to read > - * @end: output one entry > + * @ent: output one entry > * @name: if non-NULL, the name related to @dirp for use in error reporting > * > * Wrapper around readdir. Typical usage: > - * struct dirent ent; > + * struct dirent *ent; > * int rc; > * DIR *dir; > * if (virDirOpen(&dir, name) < 0) > -- > 2.12.2 > ACK (could have gone straight in as trivial :)). Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] storage: Avoid leak in virStorageUtilGlusterExtractPoolSources()
The contents of volname would be leaked if the function were
to be passed an invalid pooltype by the caller.
Make sure the memory is released instead.
---
Changes from v1:
* use VIR_STEAL_PTR() instead of open-coding it.
src/storage/storage_util.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 7cc125a..8e25984 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2846,7 +2846,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
xmlXPathContextPtr ctxt = NULL;
xmlNodePtr *nodes = NULL;
virStoragePoolSource *src = NULL;
-char *volname;
+char *volname = NULL;
size_t i;
int nnodes;
int ret = -1;
@@ -2871,12 +2871,11 @@ virStorageUtilGlusterExtractPoolSources(const char
*host,
if (pooltype == VIR_STORAGE_POOL_NETFS) {
src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;
-src->dir = volname;
+VIR_STEAL_PTR(src->dir, volname);
} else if (pooltype == VIR_STORAGE_POOL_GLUSTER) {
-src->name = volname;
-
if (VIR_STRDUP(src->dir, "/") < 0)
goto cleanup;
+VIR_STEAL_PTR(src->name, volname);
} else {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("unsupported gluster lookup"));
@@ -2894,6 +2893,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
ret = nnodes;
cleanup:
+VIR_FREE(volname);
VIR_FREE(nodes);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(doc);
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 01/12] conf: Fix possible memleak in capabilities
On Wed, Apr 05, 2017 at 04:36:24PM +0200, Martin Kletzander wrote:
> If formatting NUMA topology fails, the function returns immediatelly,
> but the buffer structure allocated on the stack references lot of
> heap-allocated memory and that would get lost in such case.
>
> Signed-off-by: Martin Kletzander
> ---
> src/conf/capabilities.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 08907aced1b9..be95c50cfb67 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -955,7 +955,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> if (caps->host.nnumaCell &&
> virCapabilitiesFormatNUMATopology(&buf, caps->host.nnumaCell,
>caps->host.numaCell) < 0)
> -return NULL;
> +goto error;
Personally, I'd more like cleanup, but looking at other XML formatting methods,
I'm fine with this as well, at least we stay consistent.
>
> for (i = 0; i < caps->host.nsecModels; i++) {
> virBufferAddLit(&buf, "\n");
> @@ -1072,6 +1072,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> return NULL;
>
Git discarded the context here, so squash this in:
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index be95c50cf..ea6d4b19d 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1069,7 +1069,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&buf, "\n");
if (virBufferCheckError(&buf) < 0)
-return NULL;
+goto error;
return virBufferContentAndReset(&buf);
> return virBufferContentAndReset(&buf);
> +
> + error:
> +virBufferFreeAndReset(&buf);
> +return NULL;
> }
>
> /* get the maximum ID of cpus in the host */
> --
> 2.12.2
>
ACK with the bit above squashed in.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Avoid leak in virStorageUtilGlusterExtractPoolSources()
On Thu, 2017-04-06 at 08:29 +0200, Peter Krempa wrote: > > If you went with this option, then VIR_STEAL_PTR(src->dir, volname); and > > VIR_STEAL_PTR(src->name, volname); instead of inlining. > > I vote for this. > > > And as some would say the comment is "obvious". > > And for this. Me too. I apparently forgot how to VIR_STEAL_PTR() :/ v2 coming right up. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
