[libvirt] [PATCH v2 0/9] Add function that raises error if domain is not active

Clementine Hayat posted 9 patches 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180417221334.13845-1-clem@lse.epita.fr
Test syntax-check passed
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(-)
[libvirt] [PATCH v2 0/9] Add function that raises error if domain is not active
Posted by Clementine Hayat 5 years, 11 months ago
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
Re: [libvirt] [PATCH v2 0/9] Add function that raises error if domain is not active
Posted by Ján Tomko 5 years, 11 months ago
On Tue, Apr 17, 2018 at 10:13:25PM +0000, Clementine Hayat wrote:
>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(-)
>

I have pushed most of the patches, except for qemu, lxc and bhyve.

On top of that, some APIs like GetVcpus or GetIOThreads use
unnecessarily specific error messages:

src/qemu/qemu_driver.c:5517:    if (!virDomainObjIsActive(vm)) {
src/qemu/qemu_driver.c-5518-        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
src/qemu/qemu_driver.c-5519-                       _("cannot list IOThreads for an inactive domain"));
src/qemu/qemu_driver.c-5520-        goto endjob;

which essentially repeat the API name in the error message, but those
can be cleaned up separately.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list