11:25 a.m.
I think we need a simple and consistent way of checking whether flags passed
to API entry points in drivers are valid/supported or not. So far some entry
points don't check flags at all and some checks for 0 when no flags are
expected/supported.
The first patch introduces a macro which can be called with a whitelist of
supported flags (more info about it inside the patch). The second patch changes
all API calls introduced in 0.8.0 release to use the macro for checking valid
flags.
I'm not sure if we can change existing API calls to do the same as it could
break some badly written apps which pass invalid flags. My opinion is that we
may often do that as such apps don't use the API in a documented/valid way.
Changes in v2:
- virCheckFlags moved from checks.h to existing internal.h
Jirka
Jiri Denemark (2):
Introduce virCheckFlags for consistent flags checking
Use virCheckFlags for APIs added in 0.8.0
src/esx/esx_driver.c | 43 ++++++++++++-------------
src/internal.h | 23 +++++++++++++
src/nwfilter/nwfilter_driver.c | 4 ++-
src/qemu/qemu_driver.c | 68 +++++++++++++++++++---------------------
src/storage/storage_driver.c | 6 +---
src/vbox/vbox_tmpl.c | 41 ++++++++++++++++-------
src/xen/xend_internal.c | 4 ++
7 files changed, 111 insertions(+), 78 deletions(-)
11:25 a.m.
New subject: [libvirt] [PATCH v2 1/2] Introduce virCheckFlags for consistent flags checking
The idea is that every API implementation in driver which has flags
parameter should first call virCheckFlags() macro to check the function
was called with supported flags:
virCheckFlags(VIR_SUPPORTED_FLAG_1 |
VIR_SUPPORTED_FLAG_2 |
VIR_ANOTHER_SUPPORTED_FLAG, -1);
The error massage which is printed when unsupported flags are passed
looks like:
invalid argument in virFooBar: unsupported flags (0x2)
Where the unsupported flags part only prints those flags which were
passed but are not supported rather than all flags passed.
---
src/internal.h | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index 2e73210..12e9a1f 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -191,4 +191,27 @@
fprintf(stderr, "Unimplemented block at %s:%d\n", \
__FILE__, __LINE__);
+/**
+ * virCheckFlags:
+ * @supported: an OR'ed set of supported flags
+ * @retval: return value in case unsupported flags were passed
+ *
+ * Returns nothing. Exits the caller function if unsupported flags were
+ * passed to it.
+ */
+# define virCheckFlags(supported, retval) \
+ do { \
+ if ((flags & ~(supported))) { \
+ virReportErrorHelper(NULL, \
+ VIR_FROM_THIS, \
+ VIR_ERR_INVALID_ARG, \
+ __FILE__, \
+ __FUNCTION__, \
+ __LINE__, \
+ _("%s: unsupported flags (0x%x)"), \
+ __FUNCTION__, flags & ~(supported)); \
+ return retval; \
+ } \
+ } while (0)
+
#endif /* __VIR_INTERNAL_H__ */
--
1.7.0.4
9:36 a.m.
New subject: [libvirt] [PATCH v2 1/2] Introduce virCheckFlags for consistent flags checking
On 04/13/2010 10:25 AM, Jiri Denemark wrote:
The idea is that every API implementation in driver which has flags
parameter should first call virCheckFlags() macro to check the function
was called with supported flags:
virCheckFlags(VIR_SUPPORTED_FLAG_1 |
VIR_SUPPORTED_FLAG_2 |
VIR_ANOTHER_SUPPORTED_FLAG, -1);
The error massage which is printed when unsupported flags are passed
looks like:
invalid argument in virFooBar: unsupported flags (0x2)
Where the unsupported flags part only prints those flags which were
passed but are not supported rather than all flags passed.
---
src/internal.h | 23 +++++++++++++++++++++++
I like this location better than the v1 attempt. ACK (but note my
comments on Matthias' ESX patch, depending on which gets pushed first).
+/**
+ * virCheckFlags:
+ * @supported: an OR'ed set of supported flags
Is it worth documenting that this must be 'int' or 'unsigned int', and
that the macro doesn't work on uint64_t? Or maybe it's worth trying to
figure out a way to refactor the macro to support both sizes? But that
can be a followup, if we find a case where we ever need a larger size
for flags, so it shouldn't hold up committing this.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
7:18 a.m.
New subject: [libvirt] [PATCH v2 1/2] Introduce virCheckFlags for consistent flags checking
> The idea is that every API implementation in driver which has
flags
> parameter should first call virCheckFlags() macro to check the function
> was called with supported flags:
>
> virCheckFlags(VIR_SUPPORTED_FLAG_1 |
> VIR_SUPPORTED_FLAG_2 |
> VIR_ANOTHER_SUPPORTED_FLAG, -1);
>
> The error massage which is printed when unsupported flags are passed
> looks like:
>
> invalid argument in virFooBar: unsupported flags (0x2)
>
> Where the unsupported flags part only prints those flags which were
> passed but are not supported rather than all flags passed.
> ---
> src/internal.h | 23 +++++++++++++++++++++++
I like this location better than the v1 attempt. ACK (but note my
comments on Matthias' ESX patch, depending on which gets pushed first).
Thanks, I pushed the patch.
> +/**
> + * virCheckFlags:
> + * @supported: an OR'ed set of supported flags
Is it worth documenting that this must be 'int' or 'unsigned int', and
that the macro doesn't work on uint64_t? Or maybe it's worth trying to
figure out a way to refactor the macro to support both sizes? But that
can be a followup, if we find a case where we ever need a larger size
for flags, so it shouldn't hold up committing this.
I left it as is for now as we only use (unsigned) int flags now. Once we add
an API with 64b flags we can enhance this macro.
Jirka
11:25 a.m.
New subject: [libvirt] [PATCH v2 2/2] Use virCheckFlags for APIs added in 0.8.0
---
src/esx/esx_driver.c | 43 ++++++++++++-------------
src/nwfilter/nwfilter_driver.c | 4 ++-
src/qemu/qemu_driver.c | 68 +++++++++++++++++++---------------------
src/storage/storage_driver.c | 6 +---
src/vbox/vbox_tmpl.c | 41 ++++++++++++++++-------
src/xen/xend_internal.c | 4 ++
6 files changed, 88 insertions(+), 78 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 4ed9890..d9a1cfd 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3296,7 +3296,7 @@ esxDomainIsPersistent(virDomainPtr domain ATTRIBUTE_UNUSED)
static virDomainSnapshotPtr
esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
esxPrivate *priv = domain->conn->privateData;
virDomainSnapshotDefPtr def = NULL;
@@ -3308,6 +3308,8 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
esxVI_TaskInfoState taskInfoState;
virDomainSnapshotPtr snapshot = NULL;
+ virCheckFlags(0, NULL);
+
if (esxVI_EnsureSession(priv->host) < 0) {
goto failure;
}
@@ -3369,7 +3371,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
static char *
esxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
esxPrivate *priv = snapshot->domain->conn->privateData;
esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL;
@@ -3379,6 +3381,8 @@ esxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot,
char uuid_string[VIR_UUID_STRING_BUFLEN] = "";
char *xml = NULL;
+ virCheckFlags(0, NULL);
+
memset(&def, 0, sizeof (virDomainSnapshotDef));
if (esxVI_EnsureSession(priv->host) < 0) {
@@ -3423,12 +3427,14 @@ esxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot,
static int
-esxDomainSnapshotNum(virDomainPtr domain, unsigned int flags ATTRIBUTE_UNUSED)
+esxDomainSnapshotNum(virDomainPtr domain, unsigned int flags)
{
int result = 0;
esxPrivate *priv = domain->conn->privateData;
esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL;
+ virCheckFlags(0, -1);
+
if (esxVI_EnsureSession(priv->host) < 0) {
goto failure;
}
@@ -3455,12 +3461,14 @@ esxDomainSnapshotNum(virDomainPtr domain, unsigned int flags
ATTRIBUTE_UNUSED)
static int
esxDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
int result = 0;
esxPrivate *priv = domain->conn->privateData;
esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL;
+ virCheckFlags(0, -1);
+
if (names == NULL || nameslen < 0) {
ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", _("Invalid argument"));
return -1;
@@ -3496,7 +3504,7 @@ esxDomainSnapshotListNames(virDomainPtr domain, char **names, int
nameslen,
static virDomainSnapshotPtr
esxDomainSnapshotLookupByName(virDomainPtr domain, const char *name,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
esxPrivate *priv = domain->conn->privateData;
esxVI_VirtualMachineSnapshotTree *rootSnapshotTreeList = NULL;
@@ -3504,6 +3512,8 @@ esxDomainSnapshotLookupByName(virDomainPtr domain, const char
*name,
esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL;
virDomainSnapshotPtr snapshot = NULL;
+ virCheckFlags(0, NULL);
+
if (esxVI_EnsureSession(priv->host) < 0) {
goto cleanup;
}
@@ -3533,12 +3543,7 @@ esxDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int
flags)
esxPrivate *priv = domain->conn->privateData;
esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL;
- if (flags != 0) {
- ESX_ERROR(VIR_ERR_INVALID_ARG,
- _("Unsupported flags (0x%x) passed to %s"),
- flags, __FUNCTION__);
- return -1;
- }
+ virCheckFlags(0, -1);
if (esxVI_EnsureSession(priv->host) < 0) {
goto failure;
@@ -3574,12 +3579,7 @@ esxDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags)
esxPrivate *priv = domain->conn->privateData;
esxVI_VirtualMachineSnapshotTree *currentSnapshotTree = NULL;
- if (flags != 0) {
- ESX_ERROR(VIR_ERR_INVALID_ARG,
- _("Unsupported flags (0x%x) passed to %s"),
- flags, __FUNCTION__);
- return NULL;
- }
+ virCheckFlags(0, NULL);
if (esxVI_EnsureSession(priv->host) < 0) {
goto cleanup;
@@ -3612,12 +3612,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned
int flags)
esxVI_ManagedObjectReference *task = NULL;
esxVI_TaskInfoState taskInfoState;
- if (flags != 0) {
- ESX_ERROR(VIR_ERR_INVALID_ARG,
- _("Unsupported flags (0x%x) passed to %s"),
- flags, __FUNCTION__);
- return -1;
- }
+ virCheckFlags(0, -1);
if (esxVI_EnsureSession(priv->host) < 0) {
goto failure;
@@ -3670,6 +3665,8 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int
flags)
esxVI_ManagedObjectReference *task = NULL;
esxVI_TaskInfoState taskInfoState;
+ virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1);
+
if (esxVI_EnsureSession(priv->host) < 0) {
goto failure;
}
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index f237b7c..35ce807 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -374,11 +374,13 @@ cleanup:
static char *
nwfilterDumpXML(virNWFilterPtr obj,
- unsigned int flags ATTRIBUTE_UNUSED) {
+ unsigned int flags) {
virNWFilterDriverStatePtr driver = obj->conn->nwfilterPrivateData;
virNWFilterPoolObjPtr pool;
char *ret = NULL;
+ virCheckFlags(0, NULL);
+
nwfilterDriverLock(driver);
pool = virNWFilterPoolObjFindByUUID(&driver->pools, obj->uuid);
nwfilterDriverUnlock(driver);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index df1d435..3981a8d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4997,12 +4997,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
int ret = -1;
int compressed;
- if (flags != 0) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- _("unsupported flags (0x%x) passed to '%s'"),
- flags, __FUNCTION__);
- return -1;
- }
+ virCheckFlags(0, -1);
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -5063,12 +5058,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int
flags)
int ret = -1;
char *name = NULL;
- if (flags != 0) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- _("unsupported flags (0x%x) passed to '%s'"),
- flags, __FUNCTION__);
- return -1;
- }
+ virCheckFlags(0, -1);
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -5102,12 +5092,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
int ret = -1;
char *name = NULL;
- if (flags != 0) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- _("unsupported flags (0x%x) passed to '%s'"),
- flags, __FUNCTION__);
- return -1;
- }
+ virCheckFlags(0, -1);
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -7630,11 +7615,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
virCgroupPtr cgroup = NULL;
int ret = -1;
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- qemuReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot modify the persistent
configuration of a domain"));
- return -1;
- }
+ virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT |
+ VIR_DOMAIN_DEVICE_MODIFY_LIVE, -1);
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -10516,11 +10498,7 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom,
qemuDomainObjPrivatePtr priv;
int ret = -1;
- if (flags != 0) {
- qemuReportError(VIR_ERR_INVALID_ARG,
- _("unsupported flags (0x%x) passed to '%s'"),
flags, __FUNCTION__);
- return -1;
- }
+ virCheckFlags(0, -1);
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -10679,7 +10657,7 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
const char *xmlDesc,
- unsigned int flags
ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm = NULL;
@@ -10691,6 +10669,8 @@ static virDomainSnapshotPtr
qemuDomainSnapshotCreateXML(virDomainPtr domain,
const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL,
NULL };
int i;
+ virCheckFlags(0, NULL);
+
qemuDriverLock(driver);
virUUIDFormat(domain->uuid, uuidstr);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
@@ -10796,12 +10776,14 @@ cleanup:
static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names,
int nameslen,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm = NULL;
int n = -1;
+ virCheckFlags(0, -1);
+
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
if (!vm) {
@@ -10822,12 +10804,14 @@ cleanup:
}
static int qemuDomainSnapshotNum(virDomainPtr domain,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm = NULL;
int n = -1;
+ virCheckFlags(0, -1);
+
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
if (!vm) {
@@ -10849,13 +10833,15 @@ cleanup:
static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain,
const char *name,
- unsigned int flags
ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm;
virDomainSnapshotObjPtr snap = NULL;
virDomainSnapshotPtr snapshot = NULL;
+ virCheckFlags(0, NULL);
+
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
if (!vm) {
@@ -10883,12 +10869,14 @@ cleanup:
}
static int qemuDomainHasCurrentSnapshot(virDomainPtr domain,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm;
int ret = -1;
+ virCheckFlags(0, -1);
+
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
if (!vm) {
@@ -10909,12 +10897,14 @@ cleanup:
}
static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain,
- unsigned int flags
ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = domain->conn->privateData;
virDomainObjPtr vm;
virDomainSnapshotPtr snapshot = NULL;
+ virCheckFlags(0, NULL);
+
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
if (!vm) {
@@ -10941,7 +10931,7 @@ cleanup:
}
static char *qemuDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = snapshot->domain->conn->privateData;
virDomainObjPtr vm = NULL;
@@ -10949,6 +10939,8 @@ static char *qemuDomainSnapshotDumpXML(virDomainSnapshotPtr
snapshot,
virDomainSnapshotObjPtr snap = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virCheckFlags(0, NULL);
+
qemuDriverLock(driver);
virUUIDFormat(snapshot->domain->uuid, uuidstr);
vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid);
@@ -10976,7 +10968,7 @@ cleanup:
}
static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
struct qemud_driver *driver = snapshot->domain->conn->privateData;
virDomainObjPtr vm = NULL;
@@ -10987,6 +10979,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
qemuDomainObjPrivatePtr priv;
int rc;
+ virCheckFlags(0, -1);
+
qemuDriverLock(driver);
virUUIDFormat(snapshot->domain->uuid, uuidstr);
vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid);
@@ -11212,6 +11206,8 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr
snapshot,
char uuidstr[VIR_UUID_STRING_BUFLEN];
struct snap_remove rem;
+ virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1);
+
qemuDriverLock(driver);
virUUIDFormat(snapshot->domain->uuid, uuidstr);
vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid);
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index d476822..2c69ba9 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1681,11 +1681,7 @@ storageVolumeWipe(virStorageVolPtr obj,
virStorageVolDefPtr vol = NULL;
int ret = -1;
- if (flags != 0) {
- virStorageReportError(VIR_ERR_INVALID_ARG,
- _("Unsupported flags (0x%x) passed to
'%s'"), flags, __FUNCTION__);
- goto out;
- }
+ virCheckFlags(0, -1);
storageDriverLock(driver);
pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a07cf8e..0ea99d1 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -4877,11 +4877,8 @@ static int vboxDomainAttachDeviceFlags(virDomainPtr dom, const char
*xml,
static int vboxDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
unsigned int flags) {
- if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
- vboxError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot modify the persistent configuration of a
domain"));
- return -1;
- }
+ virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT |
+ VIR_DOMAIN_DEVICE_MODIFY_LIVE, -1);
return vboxDomainAttachDeviceImpl(dom, xml, 1);
}
@@ -5166,7 +5163,7 @@ cleanup:
static virDomainSnapshotPtr
vboxDomainSnapshotCreateXML(virDomainPtr dom,
const char *xmlDesc,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
VBOX_OBJECT_CHECK(dom->conn, virDomainSnapshotPtr, NULL);
virDomainSnapshotDefPtr def = NULL;
@@ -5185,6 +5182,8 @@ vboxDomainSnapshotCreateXML(virDomainPtr dom,
PRInt32 result;
#endif
+ virCheckFlags(0, NULL);
+
if (!(def = virDomainSnapshotDefParseString(xmlDesc, 1)))
goto cleanup;
@@ -5282,7 +5281,7 @@ cleanup:
static char *
vboxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
virDomainPtr dom = snapshot->domain;
VBOX_OBJECT_CHECK(dom->conn, char *, NULL);
@@ -5298,6 +5297,8 @@ vboxDomainSnapshotDumpXML(virDomainSnapshotPtr snapshot,
PRBool online = PR_FALSE;
char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virCheckFlags(0, NULL);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(domiid) < 0) {
virReportOOMError();
@@ -5399,7 +5400,7 @@ no_memory:
static int
vboxDomainSnapshotNum(virDomainPtr dom,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
VBOX_OBJECT_CHECK(dom->conn, int, -1);
vboxIID *iid = NULL;
@@ -5407,6 +5408,8 @@ vboxDomainSnapshotNum(virDomainPtr dom,
nsresult rc;
PRUint32 snapshotCount;
+ virCheckFlags(0, -1);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(iid) < 0) {
virReportOOMError();
@@ -5442,7 +5445,7 @@ static int
vboxDomainSnapshotListNames(virDomainPtr dom,
char **names,
int nameslen,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
VBOX_OBJECT_CHECK(dom->conn, int, -1);
vboxIID *iid = NULL;
@@ -5452,6 +5455,8 @@ vboxDomainSnapshotListNames(virDomainPtr dom,
int count = 0;
int i;
+ virCheckFlags(0, -1);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(iid) < 0) {
virReportOOMError();
@@ -5512,7 +5517,7 @@ cleanup:
static virDomainSnapshotPtr
vboxDomainSnapshotLookupByName(virDomainPtr dom,
const char *name,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
VBOX_OBJECT_CHECK(dom->conn, virDomainSnapshotPtr, NULL);
vboxIID *iid = NULL;
@@ -5520,6 +5525,8 @@ vboxDomainSnapshotLookupByName(virDomainPtr dom,
ISnapshot *snapshot = NULL;
nsresult rc;
+ virCheckFlags(0, NULL);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(iid) < 0) {
virReportOOMError();
@@ -5549,7 +5556,7 @@ cleanup:
static int
vboxDomainHasCurrentSnapshot(virDomainPtr dom,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
VBOX_OBJECT_CHECK(dom->conn, int, -1);
vboxIID *iid = NULL;
@@ -5557,6 +5564,8 @@ vboxDomainHasCurrentSnapshot(virDomainPtr dom,
ISnapshot *snapshot = NULL;
nsresult rc;
+ virCheckFlags(0, -1);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(iid) < 0) {
virReportOOMError();
@@ -5592,7 +5601,7 @@ cleanup:
static virDomainSnapshotPtr
vboxDomainSnapshotCurrent(virDomainPtr dom,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
VBOX_OBJECT_CHECK(dom->conn, virDomainSnapshotPtr, NULL);
vboxIID *iid = NULL;
@@ -5602,6 +5611,8 @@ vboxDomainSnapshotCurrent(virDomainPtr dom,
char *name = NULL;
nsresult rc;
+ virCheckFlags(0, NULL);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(iid) < 0) {
virReportOOMError();
@@ -5763,7 +5774,7 @@ cleanup:
static int
vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
virDomainPtr dom = snapshot->domain;
VBOX_OBJECT_CHECK(dom->conn, int, -1);
@@ -5775,6 +5786,8 @@ vboxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
PRUint32 state;
nsresult rc;
+ virCheckFlags(0, -1);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(domiid) < 0) {
virReportOOMError();
@@ -5941,6 +5954,8 @@ vboxDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
PRUint32 state;
nsresult rc;
+ virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1);
+
#if VBOX_API_VERSION == 2002
if (VIR_ALLOC(domiid) < 0) {
virReportOOMError();
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index c4e73b7..d81f6e3 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4202,6 +4202,10 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
virBuffer buf = VIR_BUFFER_INITIALIZER;
char class[8], ref[80];
+ virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_CURRENT |
+ VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
+
if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__);
return -1;
--
1.7.0.4
9:41 a.m.
New subject: [libvirt] [PATCH v2 2/2] Use virCheckFlags for APIs added in 0.8.0
On 04/13/2010 10:25 AM, Jiri Denemark wrote:
---
src/esx/esx_driver.c | 43 ++++++++++++-------------
src/nwfilter/nwfilter_driver.c | 4 ++-
src/qemu/qemu_driver.c | 68 +++++++++++++++++++---------------------
src/storage/storage_driver.c | 6 +---
src/vbox/vbox_tmpl.c | 41 ++++++++++++++++-------
src/xen/xend_internal.c | 4 ++
6 files changed, 88 insertions(+), 78 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 4ed9890..d9a1cfd 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3296,7 +3296,7 @@ esxDomainIsPersistent(virDomainPtr domain ATTRIBUTE_UNUSED)
static virDomainSnapshotPtr
esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
esxPrivate *priv = domain->conn->privateData;
virDomainSnapshotDefPtr def = NULL;
@@ -3308,6 +3308,8 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
esxVI_TaskInfoState taskInfoState;
virDomainSnapshotPtr snapshot = NULL;
+ virCheckFlags(0, NULL);
I scanned through the patch, and didn't see any instances where we are
calling virCheckFlags after non-trivial work. It is something to be
aware of when using the macro in the future, since:
{
ptr *foo = somethingThatMallocs();
virCheckFlags(0, NULL);
would be a memory leak, masked because the return is hidden inside the
virCheckFlags macro.
But the alternative, of making virCheckFlags result in a bool, and
making all callers use:
if (virBadFlags(0))
return NULL;
is not as aesthetic. So:
ACK.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:50 a.m.
New subject: [libvirt] [PATCH v2 2/2] Use virCheckFlags for APIs added in 0.8.0
On Thu, Apr 15, 2010 at 08:41:07AM -0600, Eric Blake wrote:
On 04/13/2010 10:25 AM, Jiri Denemark wrote:
> ---
> src/esx/esx_driver.c | 43 ++++++++++++-------------
> src/nwfilter/nwfilter_driver.c | 4 ++-
> src/qemu/qemu_driver.c | 68 +++++++++++++++++++---------------------
> src/storage/storage_driver.c | 6 +---
> src/vbox/vbox_tmpl.c | 41 ++++++++++++++++-------
> src/xen/xend_internal.c | 4 ++
> 6 files changed, 88 insertions(+), 78 deletions(-)
>
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index 4ed9890..d9a1cfd 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -3296,7 +3296,7 @@ esxDomainIsPersistent(virDomainPtr domain ATTRIBUTE_UNUSED)
>
> static virDomainSnapshotPtr
> esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc,
> - unsigned int flags ATTRIBUTE_UNUSED)
> + unsigned int flags)
> {
> esxPrivate *priv = domain->conn->privateData;
> virDomainSnapshotDefPtr def = NULL;
> @@ -3308,6 +3308,8 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char
*xmlDesc,
> esxVI_TaskInfoState taskInfoState;
> virDomainSnapshotPtr snapshot = NULL;
>
> + virCheckFlags(0, NULL);
I scanned through the patch, and didn't see any instances where we are
calling virCheckFlags after non-trivial work. It is something to be
aware of when using the macro in the future, since:
{
ptr *foo = somethingThatMallocs();
virCheckFlags(0, NULL);
would be a memory leak, masked because the return is hidden inside the
virCheckFlags macro.
But the alternative, of making virCheckFlags result in a bool, and
making all callers use:
if (virBadFlags(0))
return NULL;
is not as aesthetic. So:
ACK.
Agreed it's the right time, and internal.h is the right place for the
macro, AC :-)
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:14 a.m.
New subject: [libvirt] [PATCH v2 2/2] Use virCheckFlags for APIs added in 0.8.0
I scanned through the patch, and didn't see any instances where
we are
calling virCheckFlags after non-trivial work. It is something to be
aware of when using the macro in the future, since:
{
ptr *foo = somethingThatMallocs();
virCheckFlags(0, NULL);
would be a memory leak, masked because the return is hidden inside the
virCheckFlags macro.
Yes. I added a note to macro documentation to make this issue more visible.
ACK.
And pushed, thanks.
Jirka
5334
days inactive
5337
days old
7 comments
3 participants
participants (3)
-
Daniel Veillard -
Eric Blake -
Jiri Denemark