Re: [libvirt] [PATCH v2 2/3] storage: Introduce iscsi_direct pool type
2018-07-24 14:24 GMT+02:00 Michal Privoznik :
> On 07/23/2018 08:43 PM, c...@lse.epita.fr wrote:
>> From: Clementine Hayat
>>
>> Introducing the pool as a noop. Integration inside the build
>> system. Implementation will be in the following commits.
>>
>> Signed-off-by: Clementine Hayat
>> ---
>> configure.ac | 6 ++-
>> m4/virt-storage-iscsi-direct.m4| 41 +++
>> src/conf/domain_conf.c | 4 ++
>> src/conf/storage_conf.c| 33 ++--
>> src/conf/storage_conf.h| 1 +
>> src/conf/virstorageobj.c | 2 +
>> src/storage/Makefile.inc.am| 22
>> src/storage/storage_backend.c | 6 +++
>> src/storage/storage_backend_iscsi_direct.c | 58 ++
>> src/storage/storage_backend_iscsi_direct.h | 6 +++
>> src/storage/storage_driver.c | 1 +
>> tools/virsh-pool.c | 3 ++
>> 12 files changed, 178 insertions(+), 5 deletions(-)
>> create mode 100644 m4/virt-storage-iscsi-direct.m4
>> create mode 100644 src/storage/storage_backend_iscsi_direct.c
>> create mode 100644 src/storage/storage_backend_iscsi_direct.h
>
> Missing documentation. I can not push these without any documentation.
> You need to document what the new type is doing, what the XML should
> look like. Also, might be worth putting some test cases into
> storagepoolxml2xmltest.
> Since you will be sending v3, can you add docs/news.xml entry (in a
> separate patch) too please?
Yes sure.
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 7396616eda..5af27a6ad2 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -30163,6 +30163,10 @@
>> virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
>>
>> break;
>>
>> +case VIR_STORAGE_POOL_ISCSI_DIRECT:
>> +def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
>> +break;
>> +
>> case VIR_STORAGE_POOL_ISCSI:
>> if (def->startupPolicy) {
>> virReportError(VIR_ERR_XML_ERROR, "%s",
>> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
>> index 5036ab9ef8..ee1586410b 100644
>> --- a/src/conf/storage_conf.c
>> +++ b/src/conf/storage_conf.c
>> @@ -62,9 +62,9 @@ VIR_ENUM_IMPL(virStoragePool,
>>VIR_STORAGE_POOL_LAST,
>>"dir", "fs", "netfs",
>>"logical", "disk", "iscsi",
>> - "scsi", "mpath", "rbd",
>> - "sheepdog", "gluster", "zfs",
>> - "vstorage")
>> + "iscsi-direct", "scsi", "mpath",
>> + "rbd", "sheepdog", "gluster",
>> + "zfs", "vstorage")
>>
>> VIR_ENUM_IMPL(virStoragePoolFormatFileSystem,
>>VIR_STORAGE_POOL_FS_LAST,
>> @@ -207,6 +207,17 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
>> .formatToString = virStoragePoolFormatDiskTypeToString,
>>}
>> },
>> +{.poolType = VIR_STORAGE_POOL_ISCSI_DIRECT,
>> + .poolOptions = {
>> + .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
>> + VIR_STORAGE_POOL_SOURCE_DEVICE |
>> + VIR_STORAGE_POOL_SOURCE_NETWORK |
>> + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
>> + },
>> + .volOptions = {
>> + .formatToString = virStoragePoolFormatDiskTypeToString,
>> + }
>> +},
>> {.poolType = VIR_STORAGE_POOL_SCSI,
>> .poolOptions = {
>> .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER),
>> @@ -803,6 +814,19 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>> goto error;
>> }
>>
>> +if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT) {
>> +if (!ret->source.initiator.iqn) {
>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("missing storage pool initiator iqn"));
>> +goto error;
>> +}
>> +if (!ret->source.ndevice) {
>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>> + _("missing storage pool device path"));
>> +
[libvirt] [RFC] proposal for libiscsi storage pool
Hi everybody! I am starting this thread to discuss a new storage pool backend for iSCSI using libiSCSI. There already is an iSCSI backend, however, it uses iscsiadm binary to execute the desired operation. The binary can be spawned multiple times during single execution of an API. This is suboptimal. Moreover the iscsi storage pool is mapped by the kernel into a block device in /dev/. Iscsiadm makes operations directly on that block device. Libiscsi on the other hand is sending the commands directly to a remote iscsi portal. According to that, to be able to use a storage pool using libiscsi we have to implement the storage pool backend entirely. What we would have: Pool XML using iscsiadm: virtimages /dev/disk/by-path Pool XML using libiscsi: virtimages The change that occurs is having a direct mode that will lead to the libiscsi backend and the host mode that will lead to the actual backend using iscsiadm. To tie the backend to the front was thinking about adding something like VIR_STORAGE_POOL_LIBISCSI to storage_conf. About the domain XML only accept: would be great using a switch case on VIR_STORAGE_POOL_LIBISCSI inside domain_conf. Best regards, -- Clementine Hayat -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 8/9] uml: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/uml/uml_driver.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index ac168ce77..56dfd7b58 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -2584,11 +2584,8 @@ umlDomainOpenConsole(virDomainPtr dom,
if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (dev_name) {
for (i = 0; i < vm->def->nconsoles; i++) {
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 7/9] openvz: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/openvz/openvz_driver.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 9900e8bab..66e589313 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -575,11 +575,8 @@ static int openvzDomainSuspend(virDomainPtr dom)
if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
return -1;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
openvzSetProgramSentinal(prog, vm->def->name);
@@ -605,11 +602,8 @@ static int openvzDomainResume(virDomainPtr dom)
if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
return -1;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
openvzSetProgramSentinal(prog, vm->def->name);
@@ -1895,11 +1889,8 @@ openvzDomainInterfaceStats(virDomainPtr dom,
if (!(vm = openvzDomObjFromDomain(driver, dom->uuid)))
return -1;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (!(net = virDomainNetFind(vm->def, device)))
goto cleanup;
@@ -2135,11 +2126,8 @@ openvzDomainMigrateBegin3Params(virDomainPtr domain,
if (!(vm = openvzDomObjFromDomain(driver, domain->uuid)))
return NULL;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (openvzGetVEStatus(vm, &status, NULL) == -1)
goto cleanup;
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/9] qemu: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/qemu/qemu_domain.c | 5 +-
src/qemu/qemu_driver.c | 271 +
2 files changed, 56 insertions(+), 220 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7000de6a9..decbdb004 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8678,11 +8678,8 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr
driver,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
return -1;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fcd79bd71..a3c806271 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1837,11 +1837,8 @@ static int qemuDomainSuspend(virDomainPtr dom)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_SUSPEND) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
reason = VIR_DOMAIN_PAUSED_MIGRATION;
@@ -1906,11 +1903,8 @@ static int qemuDomainResume(virDomainPtr dom)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
state = virDomainObjGetState(vm, &reason);
if (state == VIR_DOMAIN_PMSUSPENDED) {
@@ -2090,11 +2084,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
useAgent = false;
}
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (useAgent) {
qemuAgentPtr agent;
@@ -2157,11 +2148,8 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
priv = vm->privateData;
qemuDomainObjEnterMonitor(driver, vm);
@@ -,11 +2210,8 @@ qemuDomainDestroyFlags(virDomainPtr dom,
if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
state = virDomainObjGetState(vm, &reason);
starting = (state == VIR_DOMAIN_PAUSED &&
@@ -2541,11 +2526,8 @@ static int qemuDomainInjectNMI(virDomainPtr domain,
unsigned int flags)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorInjectNMI(priv->mon);
@@ -2604,11 +2586,8 @@ static int qemuDomainSendKey(virDomainPtr domain,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes);
@@ -2721,11 +2700,8 @@ qemuDomainGetControlInfo(virDomainPtr dom,
if (virDomainGetControlInfoEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
priv = vm->privat
[libvirt] [PATCH v2 5/9] bhyve: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/bhyve/bhyve_driver.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 24c4a9c80..8aff0c65c 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -883,11 +883,8 @@ bhyveDomainCreateWithFlags(virDomainPtr dom,
if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is already running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
ret = virBhyveProcessStart(dom->conn, privconn, vm,
VIR_DOMAIN_RUNNING_BOOTED,
@@ -996,11 +993,8 @@ bhyveDomainDestroy(virDomainPtr dom)
if (virDomainDestroyEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
ret = virBhyveProcessStop(privconn, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
event = virDomainEventLifecycleNewFromObj(vm,
@@ -1031,11 +1025,8 @@ bhyveDomainShutdown(virDomainPtr dom)
if (virDomainShutdownEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
ret = virBhyveProcessShutdown(vm);
@@ -1062,11 +1053,8 @@ bhyveDomainOpenConsole(virDomainPtr dom,
if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (!vm->def->nserials) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/9] lxc: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/lxc/lxc_driver.c | 60 +---
1 file changed, 12 insertions(+), 48 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 4097cef93..008e41bda 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1508,11 +1508,8 @@ lxcDomainDestroyFlags(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
priv = vm->privateData;
ret = virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED);
@@ -2382,11 +2379,8 @@ lxcDomainBlockStats(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -2468,11 +2462,8 @@ lxcDomainBlockStatsFlags(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -2876,11 +2867,8 @@ lxcDomainInterfaceStats(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (!(net = virDomainNetFind(vm->def, device)))
goto endjob;
@@ -3100,11 +3088,8 @@ static int lxcDomainSuspend(virDomainPtr dom)
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
if (lxcFreezeContainer(vm) < 0) {
@@ -3155,11 +3140,8 @@ static int lxcDomainResume(virDomainPtr dom)
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
state = virDomainObjGetState(vm, NULL);
if (state == VIR_DOMAIN_RUNNING) {
@@ -3214,11 +3196,8 @@ lxcDomainOpenConsole(virDomainPtr dom,
if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (dev_name) {
for (i = 0; i < vm->def->nconsoles; i++) {
@@ -3292,11 +3271,8 @@ lxcDomainSendProcessSignal(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
/*
* XXX if the kernel has /proc/$PID/ns/pid we can
@@ -3391,11 +3367,8 @@ lxcDomainShutdownFlags(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (priv->initpid == 0) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -3474,11 +3447,8 @@ lxcDomainReboot(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
g
[libvirt] [PATCH v2 1/9] Add function that raises error if domain is not active
Add a function named virDomainObjCheckIsActive in src/conf/domain_conf.c.
It calls virDomainObjIsActive, raises error if necessary and returns.
There is a lot of occurence of this pattern and it will save 3 lines on
each call.
Signed-off-by: Clementine Hayat
---
src/conf/domain_conf.c | 11 +++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 1 +
3 files changed, 14 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d23182f18..dadb63360 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6003,6 +6003,17 @@ virDomainDefValidate(virDomainDefPtr def,
return 0;
}
+int
+virDomainObjCheckActive(virDomainObjPtr dom)
+{
+if (!virDomainObjIsActive(dom)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ return -1;
+}
+return 0;
+}
+
/**
* virDomainDeviceLoadparmIsValid
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bbaa24137..122a051b2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2722,6 +2722,8 @@ virDomainObjIsActive(virDomainObjPtr dom)
return dom->def->id != -1;
}
+int virDomainObjCheckActive(virDomainObjPtr dom);
+
int virDomainDefSetVcpusMax(virDomainDefPtr def,
unsigned int vcpus,
virDomainXMLOptionPtr xmlopt);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cab324c4d..99b5a0235 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -474,6 +474,7 @@ virDomainNostateReasonTypeFromString;
virDomainNostateReasonTypeToString;
virDomainObjAssignDef;
virDomainObjBroadcast;
+virDomainObjCheckActive;
virDomainObjCopyPersistentDef;
virDomainObjEndAPI;
virDomainObjFormat;
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/9] Add function that raises error if domain is not active
This is my GSOC patch contribution.
This change was suggested on BiteSizedTasks in the libvirt wiki[1].
in libvirt there is lots of occurences of this same pattern:
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
goto out;
}
This series replace these calls with a new function that check if the
domain is active and log directly the error. This allows to remove
almost 300 lines of code in the code base.
[1]
https://wiki.libvirt.org/page/BiteSizedTasks#Add_function_that_raises_error_if_domain_is_not_active
Changes since v2:
* renamed virDomainObjCheckIsActive into virDomainObjCheckActive
* add the remaining occurences
Clementine Hayat (9):
Add function that raises error if domain is not active
qemu: start using virDomainObjCheckActive
test: start using virDomainObjCheckActive
libxl: start using virDomainObjCheckActive
bhyve: start using virDomainObjCheckActive
lxc: start using virDomainObjCheckActive
openvz: start using virDomainObjCheckActive
uml: start using virDomainObjCheckActive
vz: start using virDomainObjCheckActive
src/bhyve/bhyve_driver.c | 20 +--
src/conf/domain_conf.c | 11 ++
src/conf/domain_conf.h | 2 +
src/libvirt_private.syms | 1 +
src/libxl/libxl_driver.c | 97 +++--
src/lxc/lxc_driver.c | 60 ++--
src/openvz/openvz_driver.c | 20 +--
src/qemu/qemu_domain.c | 5 +-
src/qemu/qemu_driver.c | 271 -
src/test/test_driver.c | 35 +
src/uml/uml_driver.c | 5 +-
src/vz/vz_driver.c | 5 +-
12 files changed, 120 insertions(+), 412 deletions(-)
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/9] libxl: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/libxl/libxl_driver.c | 97 +---
1 file changed, 21 insertions(+), 76 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8808da8db..b66a1de5f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1164,10 +1164,8 @@ libxlDomainSuspend(virDomainPtr dom)
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
if (libxl_domain_pause(cfg->ctx, vm->def->id) != 0) {
@@ -1220,10 +1218,8 @@ libxlDomainResume(virDomainPtr dom)
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
if (libxl_domain_unpause(cfg->ctx, vm->def->id) != 0) {
@@ -1278,11 +1274,8 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int
flags)
if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (flags & VIR_DOMAIN_SHUTDOWN_PARAVIRT) {
ret = libxl_domain_shutdown(cfg->ctx, vm->def->id);
@@ -1344,11 +1337,8 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags)
if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (flags & VIR_DOMAIN_REBOOT_PARAVIRT) {
ret = libxl_domain_reboot(cfg->ctx, vm->def->id);
@@ -1390,11 +1380,8 @@ libxlDomainDestroyFlags(virDomainPtr dom,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("Domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (libxlDomainDestroyInternal(driver, vm) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1797,10 +1784,8 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to,
const char *dxml,
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (libxlDoDomainSave(driver, vm, to) < 0)
goto endjob;
@@ -1925,10 +1910,8 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to,
unsigned int flags)
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (!(flags & VIR_DUMP_LIVE) &&
virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
@@ -2022,10 +2005,8 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int
flags)
if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
running"));
+if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-}
if (!vm->persistent) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot do managed save for transient domain"));
@@ -2493,10 +2474,8 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr
info, int maxinfo,
if (virDomainGetVcpusEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
[libvirt] [PATCH v2 3/9] test: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/test/test_driver.c | 35 +++
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index eec7a8292..43221e547 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1779,11 +1779,8 @@ static int testDomainDestroyFlags(virDomainPtr domain,
if (!(privdom = testDomObjFromDomain(domain)))
goto cleanup;
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
goto cleanup;
-}
testDomainShutdownState(domain, privdom, VIR_DOMAIN_SHUTOFF_DESTROYED);
event = virDomainEventLifecycleNewFromObj(privdom,
@@ -1921,11 +1918,8 @@ static int testDomainReboot(virDomainPtr domain,
if (!(privdom = testDomObjFromDomain(domain)))
goto cleanup;
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
goto cleanup;
-}
virDomainObjSetState(privdom, VIR_DOMAIN_SHUTDOWN,
VIR_DOMAIN_SHUTDOWN_USER);
@@ -2049,11 +2043,8 @@ testDomainSaveFlags(virDomainPtr domain, const char
*path,
if (!(privdom = testDomObjFromDomain(domain)))
goto cleanup;
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
goto cleanup;
-}
xml = virDomainDefFormat(privdom->def, privconn->caps,
VIR_DOMAIN_DEF_FORMAT_SECURE);
@@ -2255,11 +2246,8 @@ static int testDomainCoreDumpWithFormat(virDomainPtr
domain,
if (!(privdom = testDomObjFromDomain(domain)))
goto cleanup;
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
goto cleanup;
-}
if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
virReportSystemError(errno,
@@ -3231,11 +3219,8 @@ static int testDomainBlockStats(virDomainPtr domain,
if (!(privdom = testDomObjFromDomain(domain)))
return ret;
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
goto error;
-}
if (virDomainDiskIndexByName(privdom->def, path, false) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
@@ -3278,11 +3263,8 @@ testDomainInterfaceStats(virDomainPtr domain,
if (!(privdom = testDomObjFromDomain(domain)))
return -1;
-if (!virDomainObjIsActive(privdom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(privdom) < 0)
goto error;
-}
if (!(net = virDomainNetFind(privdom->def, device)))
goto error;
@@ -5962,11 +5944,8 @@ testDomainManagedSave(virDomainPtr dom, unsigned int
flags)
if (!(vm = testDomObjFromDomain(dom)))
return -1;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
-}
if (!vm->persistent) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 9/9] vz: start using virDomainObjCheckActive
Signed-off-by: Clementine Hayat
---
src/vz/vz_driver.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index e51d968f2..3094afccb 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3998,11 +3998,8 @@ vzDomainBlockResize(virDomainPtr domain,
if (vzEnsureDomainExists(dom) < 0)
goto cleanup;
-if (!virDomainObjIsActive(dom)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckActive(dom) < 0)
goto cleanup;
-}
if (!(disk = virDomainDiskByName(dom->def, path, false))) {
virReportError(VIR_ERR_INVALID_ARG,
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add function that raises error if domain is not active
2018-04-13 8:25 GMT+00:00 Erik Skultety : > On Fri, Apr 13, 2018 at 09:45:48AM +0200, Michal Privoznik wrote: >> On 04/13/2018 09:31 AM, Ján Tomko wrote: >> > On Thu, Apr 12, 2018 at 07:49:15PM +, Clementine Hayat wrote: >> >> Add a function named virDomainObjCheckIsActive in src/conf/domain_conf.c. >> >> It calls virDomainObjIsActive, raises error and returns. >> > >> > *raises error if necessary Yes, thank you. >> >> There is a lot of occurence of this pattern and it will save 3 lines on >> >> each call. Knowing that there is over 100 occurences, it will remove 300 >> >> lines from the code base. >> > >> > False advertising. >> > >> > If you don't want to send them all in one patch, I suggest spliting them >> > per-subdirectory: conf, qemu, libxl, vz, ... (and use that prefix in the >> > commit summary) I'll do that thank you. >> >> Signed-off-by: Clementine Hayat >> > >> > If you have any accents in your name, feel free to use them. Even danpb >> > recently decided the world is ready for UTF-8: >> > https://libvirt.org/git/?p=libvirt.git;a=search;s=Daniel+P.+Berrang%C3%A9;st=author >> > >> > >> >> --- >> >> Patch proposed for gsoc2018. >> >> >> >> src/conf/domain_conf.c | 11 + >> >> src/conf/domain_conf.h | 2 + >> >> src/libvirt_private.syms | 1 + >> >> src/qemu/qemu_driver.c | 96 +--- >> >> 4 files changed, 34 insertions(+), 76 deletions(-) >> >> >> > >> > The changes look good but the patch feels incomplete. >> >> I was just looking at this patch. Yes it is incomplete but I think that >> was the point. Give upstream taste what the patch looks like before >> diving in and changing all those 108 occurrences (I did change them >> locally). It's right, it's one of the BiteSizedTasks proposed to begin with [1] and it's asked to do it in two times. First do a small patch and have it review and then change all the occurences. I'm sorry I should have mentioned it. >> The patch looks good to me, but as Jan suggests, you can break this big >> change into smaller (=easier to review) patches. In the first you can >> just introduce the function. And then have patch per each folder. > I agree with the intention of the patch and the comments, I'd maybe change the > name slightly --> virDomainObjCheckActive (instead of *IsActive) or even > something that emphasizes a bit more that it will report error on inactive, so > that the caller doesn't have to care about that anymore - from the top of my > head "virDomainObjReportInactive"... I'm going to do that. I think virDomainObjCheckActive is a good name. For the virDomainObjReportInactive, I have the feeling that, by reading the name, people may expect that the function will return 0 if there was an error. >> Alternatively, you can write a small semantic patch [1] and use that to >> generate the diff. But this is rather advanced stuff. I'll take a look into coccinelle. It may take a bit more time thought. -- Clementine Hayat [1] https://wiki.libvirt.org/page/BiteSizedTasks#Add_function_that_raises_error_if_domain_is_not_active -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add function that raises error if domain is not active
Add a function named virDomainObjCheckIsActive in src/conf/domain_conf.c.
It calls virDomainObjIsActive, raises error and returns.
There is a lot of occurence of this pattern and it will save 3 lines on
each call. Knowing that there is over 100 occurences, it will remove 300
lines from the code base.
Signed-off-by: Clementine Hayat
---
Patch proposed for gsoc2018.
src/conf/domain_conf.c | 11 +
src/conf/domain_conf.h | 2 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 96 +---
4 files changed, 34 insertions(+), 76 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d23182f18..86d28c26a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6003,6 +6003,17 @@ virDomainDefValidate(virDomainDefPtr def,
return 0;
}
+int
+virDomainObjCheckIsActive(virDomainObjPtr dom)
+{
+if (!virDomainObjIsActive(dom)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+ return -1;
+}
+return 0;
+}
+
/**
* virDomainDeviceLoadparmIsValid
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bbaa24137..8de4c4145 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2722,6 +2722,8 @@ virDomainObjIsActive(virDomainObjPtr dom)
return dom->def->id != -1;
}
+int virDomainObjCheckIsActive(virDomainObjPtr dom);
+
int virDomainDefSetVcpusMax(virDomainDefPtr def,
unsigned int vcpus,
virDomainXMLOptionPtr xmlopt);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cab324c4d..d90df3583 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -474,6 +474,7 @@ virDomainNostateReasonTypeFromString;
virDomainNostateReasonTypeToString;
virDomainObjAssignDef;
virDomainObjBroadcast;
+virDomainObjCheckIsActive;
virDomainObjCopyPersistentDef;
virDomainObjEndAPI;
virDomainObjFormat;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fcd79bd71..22cc9bddb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3537,11 +3537,8 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path,
const char *dxml,
if (virDomainSaveFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckIsActive(vm) < 0)
goto cleanup;
-}
ret = qemuDomainSaveInternal(driver, vm, path, compressed,
compressedpath, dxml, flags);
@@ -3595,11 +3592,9 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int
flags)
if (virDomainManagedSaveEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckIsActive(vm) < 0)
goto cleanup;
-}
+
if (!vm->persistent) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot do managed save for transient domain"));
@@ -3939,11 +3934,8 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
VIR_DOMAIN_JOB_OPERATION_DUMP) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckIsActive(vm) < 0)
goto endjob;
-}
priv = vm->privateData;
priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP;
@@ -4054,11 +4046,8 @@ qemuDomainScreenshot(virDomainPtr dom,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckIsActive(vm) < 0)
goto endjob;
-}
/* Well, even if qemu allows multiple graphic cards, heads, whatever,
* screenshot command does not */
@@ -4165,11 +4154,8 @@ processWatchdogEvent(virQEMUDriverPtr driver,
goto cleanup;
}
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+if (virDomainObjCheckIsActive(vm) < 0)
goto endjob;
-}
flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0;
if ((ret = doCoreDump(driver, vm, dumpfile, flags,
@@ -10841,11 +10827,8 @@ qemuDomainBlockResize(virDomainPtr dom,
i
