Re: [libvirt] [Qemu-devel] [PULL 00/12] Ui 20180823 v3 patches
On 23 August 2018 at 10:56, Gerd Hoffmann wrote: > The following changes since commit d0092d90eb546a8bbe9e9120426c189474123797: > > Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20180820' into > staging (2018-08-20 17:41:18 +0100) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/ui-20180823-v3-pull-request > > for you to fetch changes up to 63cde1d13d0767f778a130e2c93c15ad709e1276: > > util: promote qemu_egl_rendernode_open() to libqemuutil (2018-08-23 > 11:53:26 +0200) > > > ui: misc fixes which piled up during 3.0 release freeze > > Hi; this failed to build on openbsd (the VM setup in tests/vm). Linking of all the qemu-system-foo binaries failed with: ../ui/egl-helpers.o: In function `egl_rendernode_init': ui/egl-helpers.c:153: undefined reference to `qemu_drm_rendernode_open' thanks -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] question about job handling across migration protocol phases
While investigating a bug [1] found by Xen's osstest I realized I don't quite understand how to handle modify jobs (e.g. BeginJob/EndJob) on virDomainObj across the various phases of V3 migration protocol. E.g. on the src host the Begin, Perform, and Confirm phases are performed. Should a modify job start (BeginJob) in the Begin phase and stop (EndJob) in the Confirm phase? Or should each phase, if necessary, do BeginJob/EndJob? Same question for dst host. IMO the job should be held across the phases on each host, preventing any modifications during the overall migration process. Although I do worry about orphaned jobs, e.g. a missed EndJob caused by some obscure error in the migration machinery. Regards, Jim [1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg01945.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 8/8] xml: report the filename (if any) when parsing files
On Thu, Aug 16, 2018 at 13:10:31 +0100, Daniel P. Berrangé wrote: > A generic "failed to parse xml document" message without telling us > which XML file failed is quite unhelpful. > > Signed-off-by: Daniel P. Berrangé > --- > src/util/virxml.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 7/8] cpu: split x86 map data into separate files
On Thu, Aug 16, 2018 at 13:10:30 +0100, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- ... > diff --git a/src/cpu_map/x86_qemu64.xml b/src/cpu_map/x86_qemu64.xml > new file mode 100644 > index 00..ed3b8d54e2 > --- /dev/null > +++ b/src/cpu_map/x86_qemu64.xml > @@ -0,0 +1,40 @@ > + > + > + > + > + ... > + > + > + > + > + > + > + > + > + Extra empty line. > + > diff --git a/src/cpu_map/x86_vendors.xml b/src/cpu_map/x86_vendors.xml > new file mode 100644 > index 00..418712af21 > --- /dev/null > +++ b/src/cpu_map/x86_vendors.xml > @@ -0,0 +1,4 @@ > + > + > + > + Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 6/8] cpu: split PPC64 map data into separate files
On Thu, Aug 16, 2018 at 13:10:29 +0100, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > src/cpu_map/Makefile.inc.am | 7 + > src/cpu_map/index.xml | 41 + > src/cpu_map/ppc64_POWER6.xml| 6 + > src/cpu_map/ppc64_POWER7.xml| 7 + > src/cpu_map/ppc64_POWER8.xml| 8 ++ > src/cpu_map/ppc64_POWER9.xml| 6 + > src/cpu_map/ppc64_POWERPC_e5500.xml | 6 + > src/cpu_map/ppc64_POWERPC_e6500.xml | 6 + > src/cpu_map/ppc64_vendors.xml | 4 +++ > 9 files changed, 57 insertions(+), 34 deletions(-) > create mode 100644 src/cpu_map/ppc64_POWER6.xml > create mode 100644 src/cpu_map/ppc64_POWER7.xml > create mode 100644 src/cpu_map/ppc64_POWER8.xml > create mode 100644 src/cpu_map/ppc64_POWER9.xml > create mode 100644 src/cpu_map/ppc64_POWERPC_e5500.xml > create mode 100644 src/cpu_map/ppc64_POWERPC_e6500.xml > create mode 100644 src/cpu_map/ppc64_vendors.xml Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/8] cpu: modularize the CPU map data file
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180816121031.10902-1-berra...@redhat.com Subject: [libvirt] [PATCH v2 0/8] cpu: modularize the CPU map data file === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 >From https://github.com/patchew-project/libvirt t [tag update]patchew/20180816121031.10902-1-berra...@redhat.com -> patchew/20180816121031.10902-1-berra...@redhat.com Switched to a new branch 'test' e98d035e16 xml: report the filename (if any) when parsing files 31ae56563c cpu: split x86 map data into separate files e84c293152 cpu: split PPC64 map data into separate files b540748699 cpu: move the CPU map data files into a src/cpu_map directory 1a0963016b cpu: simplify failure cleanup paths 1ae202dae7 cpu: push more parsing logic into common code 57e0e0f4e8 cpu: fix cleanup when signature parsing fails 2e7b65ede4 cpu: allow include files for CPU definition === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-8thgtv1m/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-8thgtv1m/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeername getpeername-tests
Re: [libvirt] [PATCH v2 5/8] cpu: move the CPU map data files into a src/cpu_map directory
On Thu, Aug 16, 2018 at 13:10:28 +0100, Daniel P. Berrangé wrote: > In preparation for splitting up the CPU map data file, move it into a > dedicated directory of its own. > > Signed-off-by: Daniel P. Berrangé Once syntax-check passes Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/8] cpu: simplify failure cleanup paths
On Thu, Aug 16, 2018 at 13:10:27 +0100, Daniel P. Berrangé wrote: > Get rid of the separate 'error:' label, so all code paths jump straight > to the 'cleanup:' label. > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/8] cpu: push more parsing logic into common code
On Thu, Aug 16, 2018 at 13:10:26 +0100, Daniel P. Berrangé wrote: > The x86 and ppc impls both duplicate some logic when parsing CPU > features. Change the callback signature so that this duplication can be > pushed up a level to common code. > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/8] cpu: modularize the CPU map data file
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180816121031.10902-1-berra...@redhat.com Subject: [libvirt] [PATCH v2 0/8] cpu: modularize the CPU map data file === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 >From https://github.com/patchew-project/libvirt t [tag update]patchew/20180816121031.10902-1-berra...@redhat.com -> patchew/20180816121031.10902-1-berra...@redhat.com Switched to a new branch 'test' 0a584b73c9 xml: report the filename (if any) when parsing files f1f5b51850 cpu: split x86 map data into separate files a97f55673b cpu: split PPC64 map data into separate files 31f5ddf92e cpu: move the CPU map data files into a src/cpu_map directory 2e5689180b cpu: simplify failure cleanup paths ee0428e3d9 cpu: push more parsing logic into common code 5bebed9f68 cpu: fix cleanup when signature parsing fails 5e0140660c cpu: allow include files for CPU definition === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-i7a5yb6y/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-i7a5yb6y/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeername getpeername-tests
Re: [libvirt] [PATCH v2 2/8] cpu: fix cleanup when signature parsing fails
On Thu, Aug 16, 2018 at 13:10:25 +0100, Daniel P. Berrangé wrote: > Two pieces of code accidentally jumped to the wrong label when they > failed causing incorrect cleanup, returning a partially initialized > CPU model struct. > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Jiri Denemark -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/8] cpu: allow include files for CPU definition
On Thu, Aug 16, 2018 at 13:10:24 +0100, Daniel P. Berrangé wrote:
> Allow for syntax
>
>
>
> to reference other files in the CPU database directory
>
> Signed-off-by: Daniel P. Berrangé
> ---
> src/cpu/cpu_map.c | 87 +--
> 1 file changed, 84 insertions(+), 3 deletions(-)
>
> diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
> index d263eb8cdd..bcd3e55417 100644
> --- a/src/cpu/cpu_map.c
> +++ b/src/cpu/cpu_map.c
> @@ -1,7 +1,7 @@
> /*
> * cpu_map.c: internal functions for handling CPU mapping configuration
> *
> - * Copyright (C) 2009-2010 Red Hat, Inc.
> + * Copyright (C) 2009-2018 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -70,6 +70,84 @@ static int load(xmlXPathContextPtr ctxt,
> return ret;
> }
>
> +static int
> +cpuMapLoadInclude(const char *filename,
> + cpuMapLoadCallback cb,
> + void *data)
> +{
> +xmlDocPtr xml = NULL;
> +xmlXPathContextPtr ctxt = NULL;
> +int ret = -1;
> +int element;
> +char *mapfile;
> +
> +if (!(mapfile = virFileFindResource(filename,
> +abs_topsrcdir "/src/cpu",
> +PKGDATADIR)))
> +return -1;
> +
> +VIR_DEBUG("Loading CPU map include from %s", mapfile);
> +
> +if (!(xml = virXMLParseFileCtxt(mapfile, &ctxt)))
> +goto cleanup;
> +
> +ctxt->node = xmlDocGetRootElement(xml);
> +
> +for (element = 0; element < CPU_MAP_ELEMENT_LAST; element++) {
> +if (load(ctxt, element, cb, data) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse CPU map '%s'"), mapfile);
> +goto cleanup;
> +}
> +}
> +
> +ret = 0;
> +
> + cleanup:
> +xmlXPathFreeContext(ctxt);
> +xmlFreeDoc(xml);
> +VIR_FREE(mapfile);
> +
> +return ret;
> +}
> +
> +
> +static int
> +loadIncludes(xmlXPathContextPtr ctxt,
> + cpuMapLoadCallback callback,
> + void *data)
> +{
> +int ret = -1;
> +xmlNodePtr ctxt_node;
> +xmlNodePtr *nodes = NULL;
> +int n;
> +size_t i;
> +
> +ctxt_node = ctxt->node;
> +
> +n = virXPathNodeSet("include", ctxt, &nodes);
> +if (n < 0)
> +goto cleanup;
> +
> +for (i = 0; i < n; i++) {
> +char *filename = virXMLPropString(nodes[i], "filename");
Reporting an error if filename is NULL, i.e., when the filename
attribute is missing would be nice.
> +VIR_DEBUG("Finding CPU map include '%s'", filename);
> +if (cpuMapLoadInclude(filename, callback, data) < 0) {
> +VIR_FREE(filename);
> +goto cleanup;
> +}
> +VIR_FREE(filename);
> +}
> +
> +ret = 0;
...
With the NULL check
Reviewed-by: Jiri Denemark
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] apparmor: fix ptrace rules with kernel 4.18
On Fri, 2018-08-24 at 08:12 +0200, Christian Ehrhardt wrote:
> Due to kernel upstream change 338d0be4 ("apparmor: fix ptrace read
> check")
> libvirt now hits apparmor denies like:
> apparmor="DENIED" operation="ptrace" profile="/usr/sbin/libvirtd"
> pid=4409 comm="libvirtd" requested_mask="read" denied_mask="read"
> peer="libvirt-14e92a75-7668-4b97-8f92-322fc1b9c78a"
>
> Extend the ptrace rule to also allow 'ptrace (read)' for libvirtd to
> work
> with these newer kernels.
>
> Fixes: https://bugs.launchpad.net/bugs/1788603
>
> Reported-by: Thadeu Lima de Souza Cascardo .com>
> Signed-off-by: Christian Ehrhardt
> ---
> examples/apparmor/usr.sbin.libvirtd | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/examples/apparmor/usr.sbin.libvirtd
> b/examples/apparmor/usr.sbin.libvirtd
> index 80e348b7ee..f0ffc53008 100644
> --- a/examples/apparmor/usr.sbin.libvirtd
> +++ b/examples/apparmor/usr.sbin.libvirtd
> @@ -50,10 +50,10 @@
># for --p2p migrations
>unix (send, receive) type=stream addr=none peer=(label=unconfined
> addr=none),
>
> - ptrace (trace) peer=unconfined,
> - ptrace (trace) peer=/usr/sbin/libvirtd,
> - ptrace (trace) peer=/usr/sbin/dnsmasq,
> - ptrace (trace) peer=libvirt-*,
> + ptrace (read,trace) peer=unconfined,
> + ptrace (read,trace) peer=/usr/sbin/libvirtd,
> + ptrace (read,trace) peer=/usr/sbin/dnsmasq,
> + ptrace (read,trace) peer=libvirt-*,
LGTM. +1 to apply
--
Jamie Strandboge | http://www.canonical.com
signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PULL 0/3] Ui2 20180824 patches
On 24 August 2018 at 16:36, Daniel P. Berrangé wrote: > On Fri, Aug 24, 2018 at 04:32:54PM +0100, Peter Maydell wrote: >> On 24 August 2018 at 08:32, Gerd Hoffmann wrote: >> > The following changes since commit >> > 5ccac548faf041ff5229a8e8342e3be14a34c8af: >> > >> > Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into >> > staging (2018-08-23 17:35:48 +0100) >> > >> > are available in the git repository at: >> > >> > git://git.kraxel.org/qemu tags/ui2-20180824-pull-request >> > >> > for you to fetch changes up to eb01505ea6c8330273d189acc053cc60d00bfa1a: >> > >> > ui: remove support for SDL1.2 in favour of SDL2 (2018-08-24 09:31:39 >> > +0200) >> > >> > >> > ui: remove deprecated UI frontends >> > >> > >> > >> > Daniel P. Berrangé (3): >> > ui: remove support for GTK2 in favour of GTK3 >> > ui: increase min required GTK3 version to 3.14.0 >> > ui: remove support for SDL1.2 in favour of SDL2 >> >> Doesn't the last one of these depend on our fixing the >> OpenBSD VM first ? > > Yep, i mentioned that in my posting of the patch series. Thought so; dropped from my queue of pullreqs to process. Please resubmit it when the OpenBSD VM has been fixed. (That is one of my buildtest setups so this would just fail the pullreq buildtest otherwise.) thanks -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PULL 0/3] Ui2 20180824 patches
On Fri, Aug 24, 2018 at 04:32:54PM +0100, Peter Maydell wrote: > On 24 August 2018 at 08:32, Gerd Hoffmann wrote: > > The following changes since commit 5ccac548faf041ff5229a8e8342e3be14a34c8af: > > > > Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into > > staging (2018-08-23 17:35:48 +0100) > > > > are available in the git repository at: > > > > git://git.kraxel.org/qemu tags/ui2-20180824-pull-request > > > > for you to fetch changes up to eb01505ea6c8330273d189acc053cc60d00bfa1a: > > > > ui: remove support for SDL1.2 in favour of SDL2 (2018-08-24 09:31:39 > > +0200) > > > > > > ui: remove deprecated UI frontends > > > > > > > > Daniel P. Berrangé (3): > > ui: remove support for GTK2 in favour of GTK3 > > ui: increase min required GTK3 version to 3.14.0 > > ui: remove support for SDL1.2 in favour of SDL2 > > Doesn't the last one of these depend on our fixing the > OpenBSD VM first ? Yep, i mentioned that in my posting of the patch series. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PULL 0/3] Ui2 20180824 patches
On 24 August 2018 at 08:32, Gerd Hoffmann wrote: > The following changes since commit 5ccac548faf041ff5229a8e8342e3be14a34c8af: > > Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into > staging (2018-08-23 17:35:48 +0100) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/ui2-20180824-pull-request > > for you to fetch changes up to eb01505ea6c8330273d189acc053cc60d00bfa1a: > > ui: remove support for SDL1.2 in favour of SDL2 (2018-08-24 09:31:39 +0200) > > > ui: remove deprecated UI frontends > > > > Daniel P. Berrangé (3): > ui: remove support for GTK2 in favour of GTK3 > ui: increase min required GTK3 version to 3.14.0 > ui: remove support for SDL1.2 in favour of SDL2 Doesn't the last one of these depend on our fixing the OpenBSD VM first ? thanks -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vl.c: make sure maxcpus matches topology to prevent migration failure
On Fri, 24 Aug 2018 10:53:32 -0300 Eduardo Habkost wrote: > On Fri, Aug 24, 2018 at 01:26:54PM +0200, Igor Mammedov wrote: > > On Fri, 24 Aug 2018 08:11:48 -0300 > > Eduardo Habkost wrote: > > > > > On Fri, Aug 24, 2018 at 11:13:50AM +0200, Igor Mammedov wrote: > > > > On Thu, 23 Aug 2018 18:32:41 +0200 > > > > Paolo Bonzini wrote: > > > > > > > > > On 23/08/2018 16:51, Igor Mammedov wrote: > > > > > > Topology (threads*cores*sockets) must match maxcpus to be valid, > > > > > > otherwise we could start QEMU with invalid topology that throws > > > > > > a error on migration destination side, that should not be reachable: > > > > > > Source: > > > > > > -smp 8,maxcpus=64,cores=1,threads=8,sockets=1 > > > > > > // hotplug cpus upto maxcpus > > > > > > Destination: > > > > > > -smp 64,maxcpus=64,cores=1,threads=8,sockets=1 > > > > > > qemu: cpu topology: sockets (1) * cores (1) * threads (8) < > > > > > > smp_cpus (64) > > > > This destination CLI aren't exactly correct as well since > > > > it should've been exactly the same -smp as on source + a bunch of > > > > -device cpufoo... > > > > so we can always say go fix your CLI so it won't trigger error. > > > > > > > > > The destination should have sockets=8, shouldn't it? > > > > either that or cores=8 or cores=4,sockets=2 ... > > > > > > > > > It seems to me that, at startup, you should have cpus = s*t*c and cpus > > > > > <= maxcpus. Currently we check cpus <= s*t*c <= maxcpus, which > > > > > doesn't > > > > > make much sense. > > > > I think that s*t*c should describe topology of whole machine > > > > including not yet plugged vcpus. "cpus = s*t*c" probably won't work > > > > for partially filled package case: > > > >-smp 1,cores=1,threads=8,sockets=1 > > > > cores/threads should reflect full package configuration > > > > for guest to see an expected topology. > > > > > > Oh, now I remember: that's the reason we don't enforce > > > s*t*c == smp_cpus nor s*t*c == max_cpus. > > > > > > Both "-smp 4,maxcpus=8,cores=2,threads=2,sockets=1" and > > > "-smp 4,maxcpus=8,cores=2,threads=2,sockets=2" > > > worked since maxcpus was introduced, making the semantics of > > > "sockets" unclear and hard to change without breaking existing > > > configs. > > Should we go with deprication thingy then, > > so we could make it clear in the future? > > Yes, but I'm not sure which option we should adopt > (s*t*c == smp_cpus or s*t*c == max_cpus). s*t*c == smp_cpus is wrong as one won't be able to start QEMU with partial package and then hotplug the rest when needed. [...] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] vircgroup: Extract placement validation into function
On Fri, Aug 24, 2018 at 03:45:53PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c | 52 +++- 1 file changed, 32 insertions(+), 20 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Allow inputvol when creating vol from inputvol to be encrypted
ping? Tks - John On 08/21/2018 12:23 PM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1613737 > > Details in the patches (and even more in the bz). > > John Ferlan (3): > storage: Remove secretPath from _virStorageBackendQemuImgInfo > storage: Allow for inputvol to have any format for encryption > storage: Allow inputvol to be encrypted > > src/storage/storage_util.c| 79 --- > src/storage/storage_util.h| 1 + > .../luks-convert-encrypt.argv | 11 +++ > .../luks-convert-encrypt2fileqcow2.argv | 7 ++ > .../luks-convert-encrypt2fileraw.argv | 7 ++ > .../luks-convert-qcow2.argv | 9 +++ > tests/storagevolxml2argvtest.c| 19 - > tests/storagevolxml2xmlin/vol-encrypt1.xml| 21 + > tests/storagevolxml2xmlin/vol-encrypt2.xml| 21 + > tests/storagevolxml2xmlin/vol-file-qcow2.xml | 21 + > 10 files changed, 184 insertions(+), 12 deletions(-) > create mode 100644 tests/storagevolxml2argvdata/luks-convert-encrypt.argv > create mode 100644 > tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv > create mode 100644 > tests/storagevolxml2argvdata/luks-convert-encrypt2fileraw.argv > create mode 100644 tests/storagevolxml2argvdata/luks-convert-qcow2.argv > create mode 100644 tests/storagevolxml2xmlin/vol-encrypt1.xml > create mode 100644 tests/storagevolxml2xmlin/vol-encrypt2.xml > create mode 100644 tests/storagevolxml2xmlin/vol-file-qcow2.xml > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] vircgroup: Extract controller detection into function
On Fri, Aug 24, 2018 at 03:45:52PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c | 48 +--- 1 file changed, 32 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] vircgroup: Duplicate string before modifying
On Fri, Aug 24, 2018 at 03:45:51PM +0200, Pavel Hrdina wrote: The 'mntDir' is part of 'struct mntent' as a result of getmntent_r therefor we should not mangle with it. therefore Signed-off-by: Pavel Hrdina --- src/util/vircgroup.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: Ignore nwfilter binding instantiation issues during reconnect
https://bugzilla.redhat.com/show_bug.cgi?id=1607202
It's essentially stated in the nwfilterBindingDelete that we
will allow the admin to shoot themselves in the foot by deleting
the nwfilter binding which then allows them to undefine the
nwfilter that is in use for the running guest...
However, by allowing this we cause a problem for libvirtd
restart reconnect processing which would then try to recreate
the missing binding attempting to use the deleted filter
resulting in an error and thus shutting the guest down.
So rather than keep adding virDomainConfNWFilterInstantiate
flags to "ignore" specific error conditions, modify the logic
to ignore, but VIR_WARN errors other than ignoreExists. This
will at least allow the guest to not shutdown for only nwfilter
binding errors that we can now perhaps recover from since we
have the binding create/delete capability.
Signed-off-by: John Ferlan
---
v2: https://www.redhat.com/archives/libvir-list/2018-August/msg01567.html
Differences to v2. Leave the ignoreExists bool, but just allow and
VIR_WARN other errors from virDomainConfNWFilterInstantiate. Continue
processing all filters from error point too.
src/qemu/qemu_process.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ab749389ee..61a277f468 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3160,20 +3160,29 @@ qemuProcessNotifyNets(virDomainDefPtr def)
}
}
-static int
-qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
+/* Attempt to instantiate the filters. Ignore failures because it's
+ * possible that someone deleted a filter binding and the associated
+ * filter while the guest was running and we don't want that action
+ * to cause failure to keep the guest running during the reconnection
+ * processing. Nor do we necessarily want other failures to do the
+ * same. We'll just log the error conditions other than of course
+ * ignoreExists possibility (e.g. the true flag) */
+static void
+qemuProcessFiltersInstantiate(virDomainDefPtr def)
{
size_t i;
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
if ((net->filter) && (net->ifname)) {
-if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
ignoreExists) < 0)
-return 1;
+if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
+ true) < 0) {
+VIR_WARN("filter '%s' instantiation for '%s' failed '%s'",
+ net->filter, net->ifname, virGetLastErrorMessage());
+virResetLastError();
+}
}
}
-
-return 0;
}
static int
@@ -7892,8 +7901,7 @@ qemuProcessReconnect(void *opaque)
qemuProcessNotifyNets(obj->def);
-if (qemuProcessFiltersInstantiate(obj->def, true))
-goto error;
+qemuProcessFiltersInstantiate(obj->def);
if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
goto error;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vl.c: make sure maxcpus matches topology to prevent migration failure
On Fri, Aug 24, 2018 at 10:53:32AM -0300, Eduardo Habkost wrote: > On Fri, Aug 24, 2018 at 01:26:54PM +0200, Igor Mammedov wrote: > > On Fri, 24 Aug 2018 08:11:48 -0300 > > Eduardo Habkost wrote: > > > > > On Fri, Aug 24, 2018 at 11:13:50AM +0200, Igor Mammedov wrote: > > > > On Thu, 23 Aug 2018 18:32:41 +0200 > > > > Paolo Bonzini wrote: > > > > > > > > > On 23/08/2018 16:51, Igor Mammedov wrote: > > > > > > Topology (threads*cores*sockets) must match maxcpus to be valid, > > > > > > otherwise we could start QEMU with invalid topology that throws > > > > > > a error on migration destination side, that should not be reachable: > > > > > > Source: > > > > > > -smp 8,maxcpus=64,cores=1,threads=8,sockets=1 > > > > > > // hotplug cpus upto maxcpus > > > > > > Destination: > > > > > > -smp 64,maxcpus=64,cores=1,threads=8,sockets=1 > > > > > > qemu: cpu topology: sockets (1) * cores (1) * threads (8) < > > > > > > smp_cpus (64) > > > > This destination CLI aren't exactly correct as well since > > > > it should've been exactly the same -smp as on source + a bunch of > > > > -device cpufoo... > > > > so we can always say go fix your CLI so it won't trigger error. > > > > > > > > > The destination should have sockets=8, shouldn't it? > > > > either that or cores=8 or cores=4,sockets=2 ... > > > > > > > > > It seems to me that, at startup, you should have cpus = s*t*c and cpus > > > > > <= maxcpus. Currently we check cpus <= s*t*c <= maxcpus, which > > > > > doesn't > > > > > make much sense. > > > > I think that s*t*c should describe topology of whole machine > > > > including not yet plugged vcpus. "cpus = s*t*c" probably won't work > > > > for partially filled package case: > > > >-smp 1,cores=1,threads=8,sockets=1 > > > > cores/threads should reflect full package configuration > > > > for guest to see an expected topology. > > > > > > Oh, now I remember: that's the reason we don't enforce > > > s*t*c == smp_cpus nor s*t*c == max_cpus. > > > > > > Both "-smp 4,maxcpus=8,cores=2,threads=2,sockets=1" and > > > "-smp 4,maxcpus=8,cores=2,threads=2,sockets=2" > > > worked since maxcpus was introduced, making the semantics of > > > "sockets" unclear and hard to change without breaking existing > > > configs. > > Should we go with deprication thingy then, > > so we could make it clear in the future? > > Yes, but I'm not sure which option we should adopt > (s*t*c == smp_cpus or s*t*c == max_cpus). > > Does anybody know what's the semantics expected by libvirt today? Libvirt requires s*c*t to equal the total number of possible CPUs, *not* the currently plugged number. ie Valid: 32 Invalid: 64 Test with: $ virsh edit QEMUGuest1 error: unsupported configuration: CPU topology doesn't match maximum vcpu count Failed. Try again? [y,n,i,f,?]: Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vl.c: make sure maxcpus matches topology to prevent migration failure
On Fri, Aug 24, 2018 at 01:26:54PM +0200, Igor Mammedov wrote: > On Fri, 24 Aug 2018 08:11:48 -0300 > Eduardo Habkost wrote: > > > On Fri, Aug 24, 2018 at 11:13:50AM +0200, Igor Mammedov wrote: > > > On Thu, 23 Aug 2018 18:32:41 +0200 > > > Paolo Bonzini wrote: > > > > > > > On 23/08/2018 16:51, Igor Mammedov wrote: > > > > > Topology (threads*cores*sockets) must match maxcpus to be valid, > > > > > otherwise we could start QEMU with invalid topology that throws > > > > > a error on migration destination side, that should not be reachable: > > > > > Source: > > > > > -smp 8,maxcpus=64,cores=1,threads=8,sockets=1 > > > > > // hotplug cpus upto maxcpus > > > > > Destination: > > > > > -smp 64,maxcpus=64,cores=1,threads=8,sockets=1 > > > > > qemu: cpu topology: sockets (1) * cores (1) * threads (8) < > > > > > smp_cpus (64) > > > This destination CLI aren't exactly correct as well since > > > it should've been exactly the same -smp as on source + a bunch of -device > > > cpufoo... > > > so we can always say go fix your CLI so it won't trigger error. > > > > > > > The destination should have sockets=8, shouldn't it? > > > either that or cores=8 or cores=4,sockets=2 ... > > > > > > > It seems to me that, at startup, you should have cpus = s*t*c and cpus > > > > <= maxcpus. Currently we check cpus <= s*t*c <= maxcpus, which doesn't > > > > make much sense. > > > I think that s*t*c should describe topology of whole machine > > > including not yet plugged vcpus. "cpus = s*t*c" probably won't work > > > for partially filled package case: > > >-smp 1,cores=1,threads=8,sockets=1 > > > cores/threads should reflect full package configuration > > > for guest to see an expected topology. > > > > Oh, now I remember: that's the reason we don't enforce > > s*t*c == smp_cpus nor s*t*c == max_cpus. > > > > Both "-smp 4,maxcpus=8,cores=2,threads=2,sockets=1" and > > "-smp 4,maxcpus=8,cores=2,threads=2,sockets=2" > > worked since maxcpus was introduced, making the semantics of > > "sockets" unclear and hard to change without breaking existing > > configs. > Should we go with deprication thingy then, > so we could make it clear in the future? Yes, but I'm not sure which option we should adopt (s*t*c == smp_cpus or s*t*c == max_cpus). Does anybody know what's the semantics expected by libvirt today? -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] vircgroup: Duplicate string before modifying
The 'mntDir' is part of 'struct mntent' as a result of getmntent_r
therefor we should not mangle with it.
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index df38bb77e0..45b854e864 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -350,18 +350,22 @@ virCgroupCopyMounts(virCgroupPtr group,
static int
-virCgroupResolveMountLink(char *mntDir,
+virCgroupResolveMountLink(const char *mntDir,
const char *typeStr,
virCgroupControllerPtr controller)
{
VIR_AUTOFREE(char *) linkSrc = NULL;
+VIR_AUTOFREE(char *) tmp = NULL;
char *dirName;
struct stat sb;
-dirName = strrchr(mntDir, '/');
+if (VIR_STRDUP(tmp, mntDir) < 0)
+return -1;
+
+dirName = strrchr(tmp, '/');
if (!dirName) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Missing '/' separator in cgroup mount '%s'"),
mntDir);
+ _("Missing '/' separator in cgroup mount '%s'"), tmp);
return -1;
}
@@ -369,14 +373,14 @@ virCgroupResolveMountLink(char *mntDir,
return 0;
*dirName = '\0';
-if (virAsprintf(&linkSrc, "%s/%s", mntDir, typeStr) < 0)
+if (virAsprintf(&linkSrc, "%s/%s", tmp, typeStr) < 0)
return -1;
*dirName = '/';
if (lstat(linkSrc, &sb) < 0) {
if (errno == ENOENT) {
VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s",
- typeStr, mntDir, linkSrc);
+ typeStr, tmp, linkSrc);
} else {
virReportSystemError(errno, _("Cannot stat %s"), linkSrc);
return -1;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] vircgroup: Remove obsolete sa_assert
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 4
1 file changed, 4 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 02de33f74f..64507bf8aa 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1054,10 +1054,6 @@ virCgroupMakeGroup(virCgroupPtr parent,
if (virCgroupPathOfController(group, i, "", &path) < 0)
goto error;
-/* As of Feb 2011, clang can't see that the above function
- * call did not modify group. */
-sa_assert(group->controllers[i].mountPoint);
-
VIR_DEBUG("Make controller %s", path);
if (!virFileExists(path)) {
if (!create ||
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] vircgroup: Call virCgroupRemove inside virCgroupMakeGroup
This fixes virCgroupEnableMissingControllers where virCgroupRemove
was not called in case virCgroupMakeGroup failed.
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 8646a4a479..dde9ed21a2 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1052,7 +1052,7 @@ virCgroupMakeGroup(virCgroupPtr parent,
}
if (virCgroupPathOfController(group, i, "", &path) < 0)
-return -1;
+goto error;
/* As of Feb 2011, clang can't see that the above function
* call did not modify group. */
@@ -1076,7 +1076,7 @@ virCgroupMakeGroup(virCgroupPtr parent,
virReportSystemError(errno,
_("Failed to create controller %s for
group"),
virCgroupControllerTypeToString(i));
-return -1;
+goto error;
}
}
if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint !=
NULL &&
@@ -1084,7 +1084,7 @@ virCgroupMakeGroup(virCgroupPtr parent,
STREQ(group->controllers[i].mountPoint,
group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) {
if (virCgroupCpuSetInherit(parent, group) < 0)
-return -1;
+goto error;
}
/*
* Note that virCgroupSetMemoryUseHierarchy should always be
@@ -1096,13 +1096,17 @@ virCgroupMakeGroup(virCgroupPtr parent,
STREQ(group->controllers[i].mountPoint,
group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
if (virCgroupSetMemoryUseHierarchy(group) < 0)
-return -1;
+goto error;
}
}
}
VIR_DEBUG("Done making controllers for group");
return 0;
+
+ error:
+virCgroupRemove(group);
+return -1;
}
@@ -1316,10 +1320,8 @@ virCgroupNewPartition(const char *path,
if (virCgroupNew(-1, parentPath, NULL, controllers, &parent) < 0)
goto cleanup;
-if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0) {
-virCgroupRemove(*group);
+if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0)
goto cleanup;
-}
}
ret = 0;
@@ -1389,7 +1391,6 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
*/
if (virCgroupMakeGroup(partition, *group, create,
VIR_CGROUP_MEM_HIERACHY) < 0) {
-virCgroupRemove(*group);
virCgroupFree(group);
return -1;
}
@@ -1446,7 +1447,6 @@ virCgroupNewThread(virCgroupPtr domain,
return -1;
if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) {
-virCgroupRemove(*group);
virCgroupFree(group);
return -1;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] vircgroup: Split virCgroupPathOfController into two functions
The case where we need path of any controller is only for internal use
so move it out to a different function.
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 54 ++--
src/util/vircgroup.h | 2 +-
2 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 2bc4febf23..8646a4a479 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1772,28 +1772,13 @@ virCgroupHasController(virCgroupPtr cgroup, int
controller)
int
virCgroupPathOfController(virCgroupPtr group,
- int controller,
+ unsigned int controller,
const char *key,
char **path)
{
-if (controller == -1) {
-size_t i;
-for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
-/* Reject any controller with a placement
- * of '/' to avoid doing bad stuff to the root
- * cgroup
- */
-if (group->controllers[i].mountPoint &&
-group->controllers[i].placement &&
-STRNEQ(group->controllers[i].placement, "/")) {
-controller = i;
-break;
-}
-}
-}
-if (controller == -1) {
-virReportSystemError(ENOSYS, "%s",
- _("No controllers are mounted"));
+if (controller >= VIR_CGROUP_CONTROLLER_LAST) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid controller id '%d'"), controller);
return -1;
}
@@ -3505,6 +3490,31 @@ virCgroupRemove(virCgroupPtr group)
}
+static int
+virCgroupPathOfAnyController(virCgroupPtr group,
+ const char *name,
+ char **keypath)
+{
+size_t i;
+
+for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
+/* Reject any controller with a placement
+ * of '/' to avoid doing bad stuff to the root
+ * cgroup
+ */
+if (group->controllers[i].mountPoint &&
+group->controllers[i].placement &&
+STRNEQ(group->controllers[i].placement, "/")) {
+return virCgroupPathOfController(group, i, name, keypath);
+}
+}
+
+virReportSystemError(ENOSYS, "%s",
+ _("No controllers are mounted"));
+return -1;
+}
+
+
/*
* Returns 1 if some PIDs are killed, 0 if none are killed, or -1 on error
*/
@@ -3519,7 +3529,7 @@ virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr pids)
VIR_DEBUG("group=%p path=%s signum=%d pids=%p",
group, group->path, signum, pids);
-if (virCgroupPathOfController(group, -1, "tasks", &keypath) < 0)
+if (virCgroupPathOfAnyController(group, "tasks", &keypath) < 0)
return -1;
/* PIDs may be forking as we kill them, so loop
@@ -3622,7 +3632,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
VIR_DEBUG("group=%p path=%s signum=%d pids=%p",
group, group->path, signum, pids);
-if (virCgroupPathOfController(group, -1, "", &keypath) < 0)
+if (virCgroupPathOfAnyController(group, "", &keypath) < 0)
return -1;
if ((rc = virCgroupKillInternal(group, signum, pids)) < 0)
@@ -4180,7 +4190,7 @@ virCgroupHasController(virCgroupPtr cgroup
ATTRIBUTE_UNUSED,
int
virCgroupPathOfController(virCgroupPtr group ATTRIBUTE_UNUSED,
- int controller ATTRIBUTE_UNUSED,
+ unsigned int controller ATTRIBUTE_UNUSED,
const char *key ATTRIBUTE_UNUSED,
char **path ATTRIBUTE_UNUSED)
{
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index c7fdaaede4..ee3b7c7222 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -114,7 +114,7 @@ void virCgroupFree(virCgroupPtr *group);
bool virCgroupHasController(virCgroupPtr cgroup, int controller);
int virCgroupPathOfController(virCgroupPtr group,
- int controller,
+ unsigned int controller,
const char *key,
char **path);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] vircgroup: Extract placement validation into function
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 52 +++-
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6f27c50cbd..2bc4febf23 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -630,6 +630,36 @@ virCgroupDetectPlacement(virCgroupPtr group,
}
+static int
+virCgroupValidatePlacement(virCgroupPtr group,
+ pid_t pid)
+{
+size_t i;
+
+for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
+if (!group->controllers[i].mountPoint)
+continue;
+
+if (!group->controllers[i].placement) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find placement for controller %s at
%s"),
+ virCgroupControllerTypeToString(i),
+ group->controllers[i].placement);
+return -1;
+}
+
+VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld",
+ i,
+ virCgroupControllerTypeToString(i),
+ group->controllers[i].mountPoint,
+ group->controllers[i].placement,
+ (long long) pid);
+}
+
+return 0;
+}
+
+
static int
virCgroupDetectControllers(virCgroupPtr group,
int controllers)
@@ -701,7 +731,6 @@ virCgroupDetect(virCgroupPtr group,
const char *path,
virCgroupPtr parent)
{
-size_t i;
int rc;
VIR_DEBUG("group=%p controllers=%d path=%s parent=%p",
@@ -738,25 +767,8 @@ virCgroupDetect(virCgroupPtr group,
return -1;
/* Check that for every mounted controller, we found our placement */
-for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
-if (!group->controllers[i].mountPoint)
-continue;
-
-if (!group->controllers[i].placement) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not find placement for controller %s at
%s"),
- virCgroupControllerTypeToString(i),
- group->controllers[i].placement);
-return -1;
-}
-
-VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld",
- i,
- virCgroupControllerTypeToString(i),
- group->controllers[i].mountPoint,
- group->controllers[i].placement,
- (long long) pid);
-}
+if (virCgroupValidatePlacement(group, pid) < 0)
+return -1;
return 0;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] vircgroup: Simplify if conditions in virCgroupMakeGroup
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index dde9ed21a2..02de33f74f 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1079,24 +1079,20 @@ virCgroupMakeGroup(virCgroupPtr parent,
goto error;
}
}
-if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint !=
NULL &&
-(i == VIR_CGROUP_CONTROLLER_CPUSET ||
- STREQ(group->controllers[i].mountPoint,
-
group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) {
-if (virCgroupCpuSetInherit(parent, group) < 0)
-goto error;
+if (i == VIR_CGROUP_CONTROLLER_CPUSET &&
+group->controllers[i].mountPoint != NULL &&
+virCgroupCpuSetInherit(parent, group) < 0) {
+goto error;
}
/*
* Note that virCgroupSetMemoryUseHierarchy should always be
* called prior to creating subcgroups and attaching tasks.
*/
if ((flags & VIR_CGROUP_MEM_HIERACHY) &&
-(group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint
!= NULL) &&
-(i == VIR_CGROUP_CONTROLLER_MEMORY ||
- STREQ(group->controllers[i].mountPoint,
-
group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint))) {
-if (virCgroupSetMemoryUseHierarchy(group) < 0)
-goto error;
+i == VIR_CGROUP_CONTROLLER_MEMORY &&
+group->controllers[i].mountPoint != NULL &&
+virCgroupSetMemoryUseHierarchy(group) < 0) {
+goto error;
}
}
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] vircgroup: Extract controller detection into function
Signed-off-by: Pavel Hrdina
---
src/util/vircgroup.c | 48 +---
1 file changed, 32 insertions(+), 16 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 45b854e864..6f27c50cbd 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -631,24 +631,11 @@ virCgroupDetectPlacement(virCgroupPtr group,
static int
-virCgroupDetect(virCgroupPtr group,
-pid_t pid,
-int controllers,
-const char *path,
-virCgroupPtr parent)
+virCgroupDetectControllers(virCgroupPtr group,
+ int controllers)
{
size_t i;
size_t j;
-VIR_DEBUG("group=%p controllers=%d path=%s parent=%p",
- group, controllers, path, parent);
-
-if (parent) {
-if (virCgroupCopyMounts(group, parent) < 0)
-return -1;
-} else {
-if (virCgroupDetectMounts(group) < 0)
-return -1;
-}
if (controllers >= 0) {
VIR_DEBUG("Filtering controllers %d", controllers);
@@ -703,8 +690,37 @@ virCgroupDetect(virCgroupPtr group,
}
}
+return controllers;
+}
+
+
+static int
+virCgroupDetect(virCgroupPtr group,
+pid_t pid,
+int controllers,
+const char *path,
+virCgroupPtr parent)
+{
+size_t i;
+int rc;
+
+VIR_DEBUG("group=%p controllers=%d path=%s parent=%p",
+ group, controllers, path, parent);
+
+if (parent) {
+if (virCgroupCopyMounts(group, parent) < 0)
+return -1;
+} else {
+if (virCgroupDetectMounts(group) < 0)
+return -1;
+}
+
+rc = virCgroupDetectControllers(group, controllers);
+if (rc < 0)
+return -1;
+
/* Check that at least 1 controller is available */
-if (!controllers) {
+if (rc == 0) {
virReportSystemError(ENXIO, "%s",
_("At least one cgroup controller is required"));
return -1;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] cgroup cleanup
Another small set of cgroup cleanup patches as preparation for cgroup v2. Pavel Hrdina (7): vircgroup: Duplicate string before modifying vircgroup: Extract controller detection into function vircgroup: Extract placement validation into function vircgroup: Split virCgroupPathOfController into two functions vircgroup: Call virCgroupRemove inside virCgroupMakeGroup vircgroup: Simplify if conditions in virCgroupMakeGroup vircgroup: Remove obsolete sa_assert src/util/vircgroup.c | 200 +-- src/util/vircgroup.h | 2 +- 2 files changed, 118 insertions(+), 84 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 7/7] qemu_security: Lock metadata while relabelling
On Fri, Aug 24, 2018 at 03:29:24PM +0200, Michal Privoznik wrote: > On 08/24/2018 02:53 PM, Daniel P. Berrangé wrote: > > > > > That sounds reasonable, so we don't need the _WAIT behaviour in > > virtlockd itself, as everything will wait in the secdriver instead. > > At least for now, until we modularize the startup process with the > > shim. Guess that's just one more todo item to solve for the shim > > so not the end of the world. > > Hold on, we do need _WAIT so that we mutually exclude other virtlockd-s > from other hosts fiddling with seclabels on a shared NFS. However, we > will not deadlock on a single host, that's what I'm saying. Right but later when multiple clients are permitted to connect to the same virtlockd, the API they will use has a designed in deadlock :-( Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 7/7] qemu_security: Lock metadata while relabelling
On 08/24/2018 02:53 PM, Daniel P. Berrangé wrote: > > That sounds reasonable, so we don't need the _WAIT behaviour in > virtlockd itself, as everything will wait in the secdriver instead. > At least for now, until we modularize the startup process with the > shim. Guess that's just one more todo item to solve for the shim > so not the end of the world. Hold on, we do need _WAIT so that we mutually exclude other virtlockd-s from other hosts fiddling with seclabels on a shared NFS. However, we will not deadlock on a single host, that's what I'm saying. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/10] qemu: capabilities: Detect active block commit via QMP schema probing if possible
On Thu, Aug 23, 2018 at 14:45:35 -0400, John Ferlan wrote:
>
>
> On 08/15/2018 05:18 AM, Peter Krempa wrote:
> > For versions where we can probe that the arguments are optional we can
> > perform the probing by a schema query rather than sending a separate
> > command to do so.
> >
> > Signed-off-by: Peter Krempa
> > ---
[...]
> >
>
> Until I looked at the history of qapi/block-core.json, the "*" didn't
> make sense. Still, it seems "top" means required argument "top" while
> "*top" means optional argument "top". Does that mean "theoretically
> speaking" we could have used "*tls-creds" since that's listed as
> optional for nbd-server-start? Suffice to say screendump doesn't make
> much sense either, although in light of this "*", perhaps it too could
> be "*device"? I dunno, just guessing and grousing.
Some of them may be selected using the '*' modifier. The modifier is
meant to select optional only arguments. For arguments which are not
optional or if you don't care if it's optional no modifier still should
be used.
This is useful only in cases where some argument became optional later
on.
> Different problem for a different day, but documenting the syntax of the
> entries in the virQEMUCapsQMPSchemaQueries would be nice.
The documentation for virQEMUCapsQMPSchemaQueries says:
/* see documentation for virQEMUQAPISchemaPathGet for the query format */
static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = {
{ "blockdev-add/arg-type/options/+gluster/debug-level",
QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
{ "blockdev-add/arg-type/+gluster/debug", QEMU_CAPS_GLUSTER_DEBUG_LEVEL},
{ "blockdev-add/arg-type/+vxhs", QEMU_CAPS_VXHS},
Where we document the format of the query string:
/**
* virQEMUQAPISchemaPathGet:
* @query: string specifying the required data type (see below)
* @schema: hash table containing the schema data
* @entry: filled with the located schema object requested by @query
*
* Retrieves the requested schema entry specified by @query to @entry. The
* @query parameter has the following syntax which is very closely tied to the
* qemu schema syntax entries separated by slashes with a few special
characters:
*
* "command_or_event/attribute/subattribute/+variant_discriminator/subattribute"
*
* command_or_event: name of the event or attribute to introspect
* attribute: selects whether arguments or return type should be introspected
*("arg-type" or "ret-type" for commands, "arg-type" for events)
* subattribute: specifies member name of object types
* *subattribute: same as above but must be optional (has a property named
*'default' field in the schema)
* +variant_discriminator: In the case of unionized objects, select a
* specific case to introspect.
*
* If the name of any (sub)attribute starts with non-alphabetical symbols it
* needs to be prefixed by a single space.
*
* Array types are automatically flattened to the singular type. Alternate
* types are currently not supported.
*
* The above types can be chained arbitrarily using slashes to construct any
* path into the schema tree.
*
* Returns 0 on success (including if the requested schema was not found) and
* fills @entry appropriately. On failure returns -1 and sets an appropriate
* error message.
*/
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Allow usage of unpriv_sgio for SCSI generic hostdev
On 08/24/2018 09:02 AM, Daniel P. Berrangé wrote: > On Fri, Aug 24, 2018 at 01:32:41PM +0200, Ján Tomko wrote: >> On Thu, Aug 23, 2018 at 11:50:00AM -0400, John Ferlan wrote: >>> In what is perhaps a couple lifetimes ago at this point, patches >>> were posted and "mostly" all eventually accepted upstream to support >>> unpriv_sgio on SCSI generic hostdev devices. Since the needed parts >>> of the functionality from the kernel perspective were not yet present >>> upstream, a couple of the patches were not accepted. See: >>> >>> https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html >>> >>> and in particular patches 9 and 10. Since that time it seems things >>> have changed >> >> Nice! >> >> Do you have the kernel commit IDs? > > I've not found any sign of the "unpriv_sgio" sysfs file existing > in today's upstream LKML tree, so I'm not sure this is correct. > OK then, so much for assumptions... Well then we'll ignore this for now.. It's rather a painful to follow all the links in the private bz's on this. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] process: Ignore nwfilter binding instantiation issues during reconnect
On Fri, Aug 24, 2018 at 08:30:50AM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1607202 > > It's essentially stated in the nwfilterBindingDelete that we > will allow the admin to shoot themselves in the foot by deleting > the nwfilter binding which then allows them to undefine the > nwfilter that is in use for the running guest... > > However, by allowing this we cause a problem for libvirtd > restart reconnect processing which would then try to recreate > the missing binding attempting to use the deleted filter > resulting in an error and thus shutting the guest down. > > So rather than keep adding virDomainConfNWFilterInstantiate > flags to "ignore" specific error conditions and since (so far) > this is the only path that cared about checking if the filter > already exists and ignoring, let's just ignore all errors and > make the qemuProcessFiltersInstantiate be a void which will > attempt to check all networks for bindings and reload all filters > that exist. Using the VIR_INFO in order to at least "log" the > avoided issue. > > This also means virDomainConfNWFilterInstantiate no longer > needs to handle/check the ignoreExists possbility. > > Signed-off-by: John Ferlan > --- > > v1: https://www.redhat.com/archives/libvir-list/2018-August/msg01407.html > > Changes - removed the ignoreExists and just change the logic for > reconnect processing to essentially ignore all errors. If it's felt > the VIR_INFO would be too chatty (especially since it'll be generated > for every already defined filter binding), I can remove it. Another > option would be to keep the ignoreExists logic and only generate that > VIR_INFO for "other" messages. In that case, I'd probably want to change > it to a VIR_WARN. Still figured I'd post the remove it all option first > for consideration with this caveat so that "option" can be considered > as well. I guess the difference with 'ignore exists' is that this situation is harmless. The binding is still active and thus network traffic is confined. In the other error scenarios, the failure indicates a problem that likely means the network traffic is not confined. So it probably makes sense to keep the ignoreExists flag and use VIR_WARN for other errors. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] process: Ignore nwfilter binding instantiation issues during reconnect
On Fri, Aug 24, 2018 at 09:05:06AM -0400, John Ferlan wrote:
>
>
> On 08/24/2018 09:04 AM, Daniel P. Berrangé wrote:
> > On Fri, Aug 24, 2018 at 08:30:50AM -0400, John Ferlan wrote:
> >> +static void
> >> +qemuProcessFiltersInstantiate(virDomainDefPtr def)
> >> {
> >> size_t i;
> >>
> >> for (i = 0; i < def->nnets; i++) {
> >> virDomainNetDefPtr net = def->nets[i];
> >> if ((net->filter) && (net->ifname)) {
> >> -if (virDomainConfNWFilterInstantiate(def->name, def->uuid,
> >> net, ignoreExists) < 0)
> >> -return 1;
> >> +if (virDomainConfNWFilterInstantiate(def->name, def->uuid,
> >> net) < 0) {
> >> +VIR_INFO("filter '%s' instantiation for '%s' failed '%s'",
> >> + net->filter, net->ifname,
> >> virGetLastErrorMessage());
> >
> > Won't this cause a log message on every single running guests on every
> > libvirtd restart, since the normal scenario is that the filter binding
> > exists ?
> >
>
> Yes, that was explained under the ---
Next time I will actually read the text :-)
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] process: Ignore nwfilter binding instantiation issues during reconnect
On 08/24/2018 09:04 AM, Daniel P. Berrangé wrote:
> On Fri, Aug 24, 2018 at 08:30:50AM -0400, John Ferlan wrote:
>> +static void
>> +qemuProcessFiltersInstantiate(virDomainDefPtr def)
>> {
>> size_t i;
>>
>> for (i = 0; i < def->nnets; i++) {
>> virDomainNetDefPtr net = def->nets[i];
>> if ((net->filter) && (net->ifname)) {
>> -if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
>> ignoreExists) < 0)
>> -return 1;
>> +if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net)
>> < 0) {
>> +VIR_INFO("filter '%s' instantiation for '%s' failed '%s'",
>> + net->filter, net->ifname,
>> virGetLastErrorMessage());
>
> Won't this cause a log message on every single running guests on every
> libvirtd restart, since the normal scenario is that the filter binding
> exists ?
>
Yes, that was explained under the ---
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] process: Ignore nwfilter binding instantiation issues during reconnect
On Fri, Aug 24, 2018 at 08:30:50AM -0400, John Ferlan wrote:
> +static void
> +qemuProcessFiltersInstantiate(virDomainDefPtr def)
> {
> size_t i;
>
> for (i = 0; i < def->nnets; i++) {
> virDomainNetDefPtr net = def->nets[i];
> if ((net->filter) && (net->ifname)) {
> -if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
> ignoreExists) < 0)
> -return 1;
> +if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net)
> < 0) {
> +VIR_INFO("filter '%s' instantiation for '%s' failed '%s'",
> + net->filter, net->ifname, virGetLastErrorMessage());
Won't this cause a log message on every single running guests on every
libvirtd restart, since the normal scenario is that the filter binding
exists ?
> +virResetLastError();
> +}
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Allow usage of unpriv_sgio for SCSI generic hostdev
On Fri, Aug 24, 2018 at 01:32:41PM +0200, Ján Tomko wrote: > On Thu, Aug 23, 2018 at 11:50:00AM -0400, John Ferlan wrote: > > In what is perhaps a couple lifetimes ago at this point, patches > > were posted and "mostly" all eventually accepted upstream to support > > unpriv_sgio on SCSI generic hostdev devices. Since the needed parts > > of the functionality from the kernel perspective were not yet present > > upstream, a couple of the patches were not accepted. See: > > > > https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html > > > > and in particular patches 9 and 10. Since that time it seems things > > have changed > > Nice! > > Do you have the kernel commit IDs? I've not found any sign of the "unpriv_sgio" sysfs file existing in today's upstream LKML tree, so I'm not sure this is correct. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 7/7] qemu_security: Lock metadata while relabelling
On Fri, Aug 24, 2018 at 02:01:52PM +0200, Michal Privoznik wrote:
> On 08/23/2018 06:14 PM, Michal Privoznik wrote:
> > On 08/23/2018 05:51 PM, Daniel P. Berrangé wrote:
> >> On Thu, Aug 23, 2018 at 05:45:42PM +0200, Michal Privoznik wrote:
> >>> On 08/23/2018 05:01 PM, Daniel P. Berrangé wrote:
> On Thu, Aug 23, 2018 at 04:54:01PM +0200, Michal Privoznik wrote:
> > On 08/20/2018 07:17 PM, Michal Prívozník wrote:
> >> On 08/20/2018 05:16 PM, Daniel P. Berrangé wrote:
> >>> On Tue, Aug 14, 2018 at 01:19:43PM +0200, Michal Privoznik wrote:
> Fortunately, we have qemu wrappers so it's sufficient to put
> lock/unlock call only there.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_security.c | 107
> +++
> 1 file changed, 107 insertions(+)
>
> >
> >
> >
> >
> >>
> >> Oh right, yes, that kills the idea of using a WAIT flag for lockspaces,
> >> unless we want to introduce threads ingto virtlockd, but we can't do
> >> that because Linux has totally fubard locking across execve() :-(
>
> Actually, it's problem with open() + close() not execve(). And no dumy
> implementation (as I'm suggesting below) will not help us with that.
No, I really do mean execve() here. The open() + close() issue is the
awful standards compliant behaviour. The execve() issue with threads
is a trainwreck of Linuxs own making:
https://bugzilla.redhat.com/show_bug.cgi?id=1552621
> > But we need WAIT. I guess we do not want to do:
> >
> > lockForMetadata(const char *path) {
> > int retries;
> >
> > while (retries) {
> > rc = try_lock(path, wait=false);
> >
> > if (!rc) break;
> > if (errno = EAGAIN && retries--) {sleep(.1); continue;}
> >
> > errorOut();
> > }
> > }
> >
> > However, if we make sure that virtlockd never forks() nor execs() we
> > have nothing to fear about. Or am I missing something? And to be 100%
> > sure we can always provide dummy impl for fork() + execve() in
> > src/locking/lock_daemon.c which fails everytime.
>
> I work around this by putting a mutex into secdriver so that only one
> thread can do lock(). The others have to wait until the thread unlocks()
> the path. This way we leave virtlockd with just one thread. In my
> testing everything works like charm.
That sounds reasonable, so we don't need the _WAIT behaviour in
virtlockd itself, as everything will wait in the secdriver instead.
At least for now, until we modularize the startup process with the
shim. Guess that's just one more todo item to solve for the shim
so not the end of the world.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] process: Ignore nwfilter binding instantiation issues during reconnect
https://bugzilla.redhat.com/show_bug.cgi?id=1607202
It's essentially stated in the nwfilterBindingDelete that we
will allow the admin to shoot themselves in the foot by deleting
the nwfilter binding which then allows them to undefine the
nwfilter that is in use for the running guest...
However, by allowing this we cause a problem for libvirtd
restart reconnect processing which would then try to recreate
the missing binding attempting to use the deleted filter
resulting in an error and thus shutting the guest down.
So rather than keep adding virDomainConfNWFilterInstantiate
flags to "ignore" specific error conditions and since (so far)
this is the only path that cared about checking if the filter
already exists and ignoring, let's just ignore all errors and
make the qemuProcessFiltersInstantiate be a void which will
attempt to check all networks for bindings and reload all filters
that exist. Using the VIR_INFO in order to at least "log" the
avoided issue.
This also means virDomainConfNWFilterInstantiate no longer
needs to handle/check the ignoreExists possbility.
Signed-off-by: John Ferlan
---
v1: https://www.redhat.com/archives/libvir-list/2018-August/msg01407.html
Changes - removed the ignoreExists and just change the logic for
reconnect processing to essentially ignore all errors. If it's felt
the VIR_INFO would be too chatty (especially since it'll be generated
for every already defined filter binding), I can remove it. Another
option would be to keep the ignoreExists logic and only generate that
VIR_INFO for "other" messages. In that case, I'd probably want to change
it to a VIR_WARN. Still figured I'd post the remove it all option first
for consideration with this caveat so that "option" can be considered
as well.
src/conf/domain_nwfilter.c | 15 +++
src/conf/domain_nwfilter.h | 3 +--
src/lxc/lxc_process.c | 2 +-
src/qemu/qemu_hotplug.c| 4 ++--
src/qemu/qemu_interface.c | 4 ++--
src/qemu/qemu_process.c| 23 +++
src/uml/uml_conf.c | 2 +-
7 files changed, 25 insertions(+), 28 deletions(-)
diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
index f39c8a1f9b..51c9063ca7 100644
--- a/src/conf/domain_nwfilter.c
+++ b/src/conf/domain_nwfilter.c
@@ -84,8 +84,7 @@ virNWFilterBindingDefForNet(const char *vmname,
int
virDomainConfNWFilterInstantiate(const char *vmname,
const unsigned char *vmuuid,
- virDomainNetDefPtr net,
- bool ignoreExists)
+ virDomainNetDefPtr net)
{
virConnectPtr conn = virGetConnectNWFilter();
virNWFilterBindingDefPtr def = NULL;
@@ -93,20 +92,12 @@ virDomainConfNWFilterInstantiate(const char *vmname,
char *xml = NULL;
int ret = -1;
-VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d",
- vmname, NULLSTR(net->ifname), NULLSTR(net->filter),
ignoreExists);
+VIR_DEBUG("vmname=%s portdev=%s filter=%s",
+ vmname, NULLSTR(net->ifname), NULLSTR(net->filter));
if (!conn)
goto cleanup;
-if (ignoreExists) {
-binding = virNWFilterBindingLookupByPortDev(conn, net->ifname);
-if (binding) {
-ret = 0;
-goto cleanup;
-}
-}
-
if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
goto cleanup;
diff --git a/src/conf/domain_nwfilter.h b/src/conf/domain_nwfilter.h
index 6bda228fc8..d2ebeff853 100644
--- a/src/conf/domain_nwfilter.h
+++ b/src/conf/domain_nwfilter.h
@@ -25,8 +25,7 @@
int virDomainConfNWFilterInstantiate(const char *vmname,
const unsigned char *vmuuid,
- virDomainNetDefPtr net,
- bool ignoreExists);
+ virDomainNetDefPtr net);
void virDomainConfNWFilterTeardown(virDomainNetDefPtr net);
void virDomainConfVMNWFilterTeardown(virDomainObjPtr vm);
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 33c806630b..86f7463e53 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -303,7 +303,7 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm,
}
if (net->filter &&
-virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net, false) < 0)
+virDomainConfNWFilterInstantiate(vm->name, vm->uuid, net) < 0)
goto cleanup;
ret = containerVeth;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0b84a503bb..38c74bd9b1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3435,7 +3435,7 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm,
if (newdev->filter &&
virDomainConfNWFilterInstantiate(vm->def->name,
- vm->def->uuid, newdev, false) < 0) {
+ vm->def->uuid, newdev) < 0) {
Re: [libvirt] [PATCH v3] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
On Fri, Aug 24, 2018 at 12:54:40PM +0200, Katerina Koukiou wrote:
This patch ensures that changes in attributes of interfaces will be emit
s/will be/will/
errors accept if they are missing from the XML.
Previously we were falsely reporting successful updates, because some
changed attributes got overwritten before the validity checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1599513
Signed-off-by: Katerina Koukiou
---
Changes from v2:
* Added check for type element in info struct.
* Moved the addr checks at start the the section with info checks.
src/qemu/qemu_hotplug.c | 34 ++
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0b84a503bb..f9805627b7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3598,16 +3598,22 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
-/* info: if newdev->info is empty, fill it in from olddev,
- * otherwise verify that it matches - nothing is allowed to
- * change. (There is no helper function to do this, so
- * individually check the few feidls of virDomainDeviceInfo that
- * are relevant in this case).
+/* info: Nothing is allowed to change. First fill the missing newdev->info
+ * from olddev and then check for changes.
*/
+
+/* if addr type is missing overwrite if from olddev */
+if (newdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+newdev->info.type = olddev->info.type;
info.type and info.addr are tied together - if you copy the type, you
should copy the address too. (which was done by virDomainDeviceInfoCopy
in the old code). Copying the address conditionally below is asking for
trouble in case we omit some code path where we only copy the type
without the address.
+if (olddev->info.type != newdev->info.type) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot modify network device type"));
*network device address type
+}
+
+/* if pci addr is missing or is invalid we overwrite it from olddev */
if (!virDomainDeviceAddressIsValid(&newdev->info,
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
-goto cleanup;
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+newdev->info.addr.pci = olddev->info.addr.pci;
This assumes that olddev->info.type is _PCI, because
virDomainDeviceAddressIsValid
returns 0 if the types do not match. Rather than adding a check if
info.type == PCI, copy info.addr, not just info.addr.pci.
Also, both checks would look better combined, to avoid the case where we
copied 'type' but not the address:
if (new->type == NONE ||
!IsValid(new->info, new->type) {
new->type = old->type
new->addr = old->addr
}
Jano
}
if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci,
&newdev->info.addr.pci)) {
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 00/12] lcitool: Add 'build' action
On Fri, 2018-08-24 at 09:47 +0200, Erik Skultety wrote: > On Wed, Aug 22, 2018 at 11:44:15AM +0200, Andrea Bolognani wrote: > > Changes from [v2]: > > > > * rebase on top of master (dbc2de85f775) and integrate recent > > changes to build rules on the Jenkins side; > > > > * drop a commit that had already been merged in the meantime. > > > > Changes from [v1]: > > > > * rebase on top of master (985ab833be9b) and integrate recent > > changes to build rules on the Jenkins side; > > > > * build on more targets. > > > > [v2] https://www.redhat.com/archives/libvir-list/2018-August/msg01109.html > > [v1] https://www.redhat.com/archives/libvir-list/2018-August/msg00393.html > > Except for the arbitrary branch building, I think we're good and this can go > in. Thank you for the feature. Thank *you* for the review :) I've pushed the series now. I'll work on a more sensible way to build off arbitrary repositories/branches next. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't use legacy USB for RISC-V guests
The architecture is new enough that we don't need to
concern ourselves with backwards compatibility in any
capacity.
Signed-off-by: Andrea Bolognani
---
Requires
https://www.redhat.com/archives/libvir-list/2018-June/msg01223.html
to be applied (and test data to be refreshed in the process).
src/qemu/qemu_command.c | 4 +++-
tests/qemuxml2argvdata/riscv64-virt.args | 1 -
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 88d698e262..bac7c938ab 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3046,7 +3046,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT &&
!qemuDomainIsQ35(def) &&
-!qemuDomainIsARMVirt(def)) {
+!qemuDomainIsARMVirt(def) &&
+!qemuDomainIsRISCVVirt(def)) {
/* An appropriate default USB controller model should already
* have been selected in qemuDomainDeviceDefPostParse(); if
@@ -3085,6 +3086,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
if (usbcontroller == 0 &&
!qemuDomainIsQ35(def) &&
!qemuDomainIsARMVirt(def) &&
+!qemuDomainIsRISCVVirt(def) &&
!ARCH_IS_S390(def->os.arch)) {
/* We haven't added any USB controller yet, but we haven't been asked
* not to add one either. Add a legacy USB controller, unless we're
diff --git a/tests/qemuxml2argvdata/riscv64-virt.args
b/tests/qemuxml2argvdata/riscv64-virt.args
index 0e103a6755..a373ff2b92 100644
--- a/tests/qemuxml2argvdata/riscv64-virt.args
+++ b/tests/qemuxml2argvdata/riscv64-virt.args
@@ -21,7 +21,6 @@ server,nowait \
-no-shutdown \
-kernel /var/lib/libvirt/images/bbl \
-append 'console=ttyS0 ro root=/dev/vda' \
--usb \
-drive file=/var/lib/libvirt/images/stage4-disk.img,format=raw,if=none,\
id=drive-virtio-disk0 \
-device virtio-blk-device,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
\
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 8/9] tests: Add RISC-V architectures
On Thu, 2018-08-23 at 19:18 +0200, Andrea Bolognani wrote: > On Wed, 2018-08-22 at 11:15 +0200, Lubomir Rintel wrote: [...] > > create mode 100644 tests/qemuxml2startupxmloutdata/riscv64-virt.xml Oh, and this file is unnecessary (qemuxml2startup is only intended to cover some very specific use cases) so I've dropped it. The series is now pushed :) Thanks for your contribution! I'll post a patch to address the USB situation soon. -- 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 7/7] qemu_security: Lock metadata while relabelling
On 08/23/2018 06:14 PM, Michal Privoznik wrote:
> On 08/23/2018 05:51 PM, Daniel P. Berrangé wrote:
>> On Thu, Aug 23, 2018 at 05:45:42PM +0200, Michal Privoznik wrote:
>>> On 08/23/2018 05:01 PM, Daniel P. Berrangé wrote:
On Thu, Aug 23, 2018 at 04:54:01PM +0200, Michal Privoznik wrote:
> On 08/20/2018 07:17 PM, Michal Prívozník wrote:
>> On 08/20/2018 05:16 PM, Daniel P. Berrangé wrote:
>>> On Tue, Aug 14, 2018 at 01:19:43PM +0200, Michal Privoznik wrote:
Fortunately, we have qemu wrappers so it's sufficient to put
lock/unlock call only there.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_security.c | 107
+++
1 file changed, 107 insertions(+)
>
>
>
>
>>
>> Oh right, yes, that kills the idea of using a WAIT flag for lockspaces,
>> unless we want to introduce threads ingto virtlockd, but we can't do
>> that because Linux has totally fubard locking across execve() :-(
Actually, it's problem with open() + close() not execve(). And no dumy
implementation (as I'm suggesting below) will not help us with that.
>
> But we need WAIT. I guess we do not want to do:
>
> lockForMetadata(const char *path) {
> int retries;
>
> while (retries) {
> rc = try_lock(path, wait=false);
>
> if (!rc) break;
> if (errno = EAGAIN && retries--) {sleep(.1); continue;}
>
> errorOut();
> }
> }
>
> However, if we make sure that virtlockd never forks() nor execs() we
> have nothing to fear about. Or am I missing something? And to be 100%
> sure we can always provide dummy impl for fork() + execve() in
> src/locking/lock_daemon.c which fails everytime.
I work around this by putting a mutex into secdriver so that only one
thread can do lock(). The others have to wait until the thread unlocks()
the path. This way we leave virtlockd with just one thread. In my
testing everything works like charm.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] qemu: monitor: Use qemuMonitorJSONBlockJobError in qemuMonitorJSONDrivePivot
On Thu, Aug 23, 2018 at 17:38:39 -0400, John Ferlan wrote: > > > On 08/15/2018 07:52 AM, Peter Krempa wrote: > > The API deals with a block job so use the common error reporting > > function for it. > > > > Signed-off-by: Peter Krempa > > --- > > src/qemu/qemu_monitor_json.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Very strange - this got applied to qemuMonitorJSONBlockStream in my "git > am" of the patches... Gets stranger on the next patch too, but at least > this explains what happened there... Yeah. The blockdev series probably shifted some code arround (I was adding the APIs for media manipulation) and the context was not big enough to apply this correctly. I had the same problem now when I've rebased it locally. I've made sure now that it's applied at the correct point. > > Reviewed-by: John Ferlan > > John > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/8] qemu: monitor: Separate probing for active block commit
On Thu, Aug 23, 2018 at 18:11:09 -0400, John Ferlan wrote: > > > On 08/15/2018 07:52 AM, Peter Krempa wrote: > > Extract the code used to probe for the functionality so that it does not > > litter the code used for actual work. > > > > Signed-off-by: Peter Krempa > > --- > > src/qemu/qemu_monitor.c | 2 +- > > src/qemu/qemu_monitor_json.c | 58 > > ++-- > > src/qemu/qemu_monitor_json.h | 3 +++ > > 3 files changed, 39 insertions(+), 24 deletions(-) > > > > This one somehow feels related to the other series I reviewed where the > *top was or wasn't present to determine whether capability was there, > but I see that it'd still be useful - just the irony of it all. Your feelings are spot on. I was doing this commit when I realized that the detection is pointless at this point and cleaned it up right away so that we don't have to carry it around forever. The removal of the detection is orthogonal to this cleanup though and since we can't delete the detection code completely at this point thus the other patches were sent in a separate series. > > Reviewed-by: John Ferlan > > John > > > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > > index 9bc7aa9ed1..a60e78d967 100644 > > --- a/src/qemu/qemu_monitor.c > > +++ b/src/qemu/qemu_monitor.c > > NB: My gitk view shows a few lines above here the call to > qemuMonitorJSONBlockCommit has a second row of args that are not aligned > properly... Trivialities. That will be fixed later. I'm adding few new args for block-commit. That was not published yet though. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Allow usage of unpriv_sgio for SCSI generic hostdev
On Thu, Aug 23, 2018 at 11:50:00AM -0400, John Ferlan wrote: In what is perhaps a couple lifetimes ago at this point, patches were posted and "mostly" all eventually accepted upstream to support unpriv_sgio on SCSI generic hostdev devices. Since the needed parts of the functionality from the kernel perspective were not yet present upstream, a couple of the patches were not accepted. See: https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html and in particular patches 9 and 10. Since that time it seems things have changed Nice! Do you have the kernel commit IDs? Jano and this essentially reposts those two patches with some minor adjustments in logic in order to "restore" the support. There are lots of interesting tidbits in unreadable by the general public bz's, so I won't include links here. John Ferlan (2): qemu: Add ability to set sgio values for hostdev qemu: Add check for unpriv sgio for SCSI generic host device src/qemu/qemu_conf.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
This patch ensures that changes in attributes of interfaces will be emit
errors accept if they are missing from the XML.
Previously we were falsely reporting successful updates, because some
changed attributes got overwritten before the validity checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1599513
Signed-off-by: Katerina Koukiou
---
Changes from v2:
* Added check for type element in info struct.
* Moved the addr checks at start the the section with info checks.
src/qemu/qemu_hotplug.c | 34 ++
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0b84a503bb..f9805627b7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3598,16 +3598,22 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
-/* info: if newdev->info is empty, fill it in from olddev,
- * otherwise verify that it matches - nothing is allowed to
- * change. (There is no helper function to do this, so
- * individually check the few feidls of virDomainDeviceInfo that
- * are relevant in this case).
+/* info: Nothing is allowed to change. First fill the missing newdev->info
+ * from olddev and then check for changes.
*/
+
+/* if addr type is missing overwrite if from olddev */
+if (newdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+newdev->info.type = olddev->info.type;
+if (olddev->info.type != newdev->info.type) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot modify network device type"));
+}
+
+/* if pci addr is missing or is invalid we overwrite it from olddev */
if (!virDomainDeviceAddressIsValid(&newdev->info,
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
-goto cleanup;
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
+newdev->info.addr.pci = olddev->info.addr.pci;
}
if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci,
&newdev->info.addr.pci)) {
@@ -3622,21 +3628,33 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
/* device alias is checked already in virDomainDefCompatibleDevice */
+if (newdev->info.rombar == VIR_TRISTATE_BOOL_ABSENT)
+newdev->info.rombar = olddev->info.rombar;
if (olddev->info.rombar != newdev->info.rombar) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom bar setting"));
goto cleanup;
}
+
+if (!newdev->info.romfile &&
+VIR_STRDUP(newdev->info.romfile, olddev->info.romfile) < 0)
+goto cleanup;
if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network rom file"));
goto cleanup;
}
+
+if (newdev->info.bootIndex == 0)
+newdev->info.bootIndex = olddev->info.bootIndex;
if (olddev->info.bootIndex != newdev->info.bootIndex) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device boot index setting"));
goto cleanup;
}
+
+if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT)
+newdev->info.romenabled = olddev->info.romenabled;
if (olddev->info.romenabled != newdev->info.romenabled) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom enabled setting"));
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virsh: Implement virNodeGetSEVInfo in virsh
On Tue, Aug 21, 2018 at 11:20:27AM +0800, Han Han wrote:
> Add sub-command nodesevinfo to get node infomation of AMD SEV feature.
>
> Signed-off-by: Han Han
> ---
> tools/virsh-host.c | 66 ++
> tools/virsh.pod| 5
> 2 files changed, 71 insertions(+)
>
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 16f504bafe..0bcd71a2b8 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -952,6 +952,67 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd)
> return ret;
> }
>
> +/*
> + * "nodesevinfo" command
> + */
> +static const vshCmdInfo info_nodesevinfo[] = {
> +{.name = "help",
> + .data = N_("AMD SEV feature information.")
s/feature/platform
Note that this is part of the "node" subsystem, which means it is always going
to be tied to the firmware on a given node, thus "platform data".
> +},
> +{.name = "desc",
> + .data = N_("Returns information of SEV feature about the node.")
Returns SEV platform-specific data.
> +},
> +{.name = NULL}
> +};
> +
> +static bool
> +cmdNodesevinfo(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
s/sev/SEV/
> +{
> +virTypedParameterPtr params = NULL;
> +int nparams = 0;
> +unsigned int flags = 0;
> +bool ret = false;
> +size_t i;
> +virshControlPtr priv = ctl->privData;
> +
> +if (nparams == 0) {
> +/* Get the number of SEV info parameters */
> +if (virNodeGetSEVInfo(priv->conn, NULL, &nparams, flags) != 0) {
Have you actually tried the patches? Because ^this causes virsh to segfault
because this is not a legacy API where you need to query the number of params
first and then pre-allocate the pointer to store the data. We don't need that
anymore, remote driver is already able to allocate the memory for the caller.
> +vshError(ctl, "%s",
> + _("Unable to get number of SEV info parameters"));
> +goto cleanup;
> +}
> +}
> +
> +if (nparams == 0) {
> +ret = true;
> +goto cleanup;
> +}
> +
> +/* Now get all the SEV info parameters */
> +params = vshCalloc(ctl, nparams, sizeof(params));
no need for ^this...
> +if (virNodeGetSEVInfo(priv->conn, ¶ms, &nparams, flags) != 0) {
> +vshError(ctl, "%s", _("Unable to get SEV info parameters"));
> +goto cleanup;
> +}
> +
> +/* XXX: Need to sort the returned params once new parameter
> + * fields not of shared memory are added.
I'm not sure I see a problem ^here, can you elaborate please?
> + */
> +vshPrint(ctl, _("SEV info:\n"));
> +for (i = 0; i < nparams; i++) {
> +char *str = vshGetTypedParamValue(ctl, ¶ms[i]);
> +vshPrint(ctl, "\t%-15s %s\n", params[i].field, str);
> +VIR_FREE(str);
> +}
> +
> +ret = true;
> +
> + cleanup:
> +virTypedParamsFree(params, nparams);
> +return ret;
> +}
> +
> /*
> * "nodesuspend" command
> */
> @@ -1900,6 +1961,11 @@ const vshCmdDef hostAndHypervisorCmds[] = {
> .info = info_nodememstats,
> .flags = 0
> },
> +{.name = "nodesevinfo",
> + .handler = cmdNodesevinfo,
> + .info = info_nodesevinfo,
> + .flags = 0
> +},
> {.name = "nodesuspend",
> .handler = cmdNodeSuspend,
> .opts = opts_node_suspend,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 4e118851f8..ea513c0acc 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -317,6 +317,11 @@ of cpu statistics during 1 second.
> Returns memory stats of the node.
> If I is specified, this will print the specified cell statistics only.
>
> +=item B
> +
> +Display AMD's SEV feature of this host, including PDH, cert-chain, cbitpos
> +and reduced-phys-bits.
Returns AMD's SEV platform-specific data. Availability of the fields depends on
the version of the SEV firmware.
Explanation of fields:
pdh - Platform Diffie-Hellman key
cert-chain - certificate chain used to verify authenticity of the platform
cbitpos - C-bit position, i.e. which physical address bit marks protection
on memory pages
reduced-phys-bits - how many physical address bits we lost due to memory
encryption
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: Add --shrink to qemu-img command when shrinking vol
On Fri, Aug 17, 2018 at 04:01:32PM -0400, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1613746 > > When shrinking the capacity of a qcow2 or luks volume using > the qemu-img program, the --shrink qualifier must be added. > > Signed-off-by: John Ferlan > --- > src/storage/storage_util.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Handle opening for session
On Thu, Aug 23, 2018 at 08:54:53AM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1608275
>
> Commit id 2870419eb (in part) added virGetConnectNWFilter to
> allow opening drivers (interface, network, nwfilter, nodedev,
> secret, and storage) based on context and commit id f14c37ce4c
> started using the API; however, the nwfilterConnectOpen did
> not handle session mode resulting in the following message
> being logged when virDomainConfVMNWFilterTeardown was called
> during the domain shutdown processing:
>
> error : nwfilterConnectOpen:383 : internal error: unexpected
> nwfilter URI path '/session', try nwfilter:///system
>
> So similar to the other drivers add code in to check for
> /session when not privileged.
>
> Signed-off-by: John Ferlan
> ---
> src/nwfilter/nwfilter_driver.c | 19 ++-
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index ac3a964388..6c25293fd9 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -377,11 +377,20 @@ nwfilterConnectOpen(virConnectPtr conn,
> return VIR_DRV_OPEN_ERROR;
> }
>
> -if (STRNEQ(conn->uri->path, "/system")) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unexpected nwfilter URI path '%s', try
> nwfilter:///system"),
> - conn->uri->path);
> -return VIR_DRV_OPEN_ERROR;
> +if (driver->privileged) {
> +if (STRNEQ(conn->uri->path, "/system")) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected nwfilter URI path '%s', try
> nwfilter:///system"),
> + conn->uri->path);
> +return VIR_DRV_OPEN_ERROR;
> +}
> +} else {
> +if (STRNEQ(conn->uri->path, "/session")) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected nwfilter URI path '%s', try
> nwfilter:///session"),
> + conn->uri->path);
> +return VIR_DRV_OPEN_ERROR;
> +}
> }
This isn't right - we should never open the driver in session mode - the
nwfilterStateInitialize() method explicitly skips initialization in an
unprivileged daemon because sesson mode is not supported.
So I think we need to change the virt drivers to not blindly run this
cleanup code in session mode.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 09/12] lcitool: Add 'build' action
On Wed, Aug 22, 2018 at 11:44:24AM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Add extra verbiage for binding create/delete
On Wed, Aug 22, 2018 at 06:46:03PM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1609454
>
> Add some cautionary words related to the create and delete
> NWFilter Binding use cases and possible issues that may result
> to the virsh nwfilter-binding-{create|delete} descriptions
> and the virNWFilterBinding{CreateXML|Delete) API descriptions.
>
> Essentially summarizing commit 2d9318b6c without using the
> shoot yourself in the foot wording.
>
> Signed-off-by: John Ferlan
Reviewed-by: Daniel P. Berrangé
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 0/3] vsh: Introduce new API for printing tables
On Fri, Aug 24, 2018 at 12:10:47PM +0200, Michal Privoznik wrote: > On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote: > > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote: > >> On 08/23/2018 05:53 PM, Simon Kobyda wrote: > >>> Created new API for priting tables, mainly to solve alignment problems. > >>> Implemented these test to virsh list. In the future, API may be > >>> everywhere in virsh and virt-admin. > >>> Also wrote basic tests for the new API, and corrected tests in virshtest > >>> which are influenced by implementation of the API in virsh list. > >>> > >>> Changes in v5: > >>> - cleanup and merged code for calculating zero-width, nonprintable and > >>> combined > >>> character. > >>> - replaced virBufferAddStr with virBufferAddChar in some places > >>> - in tests moved code for setting correct locale > >>> - fixed few leaks and unitialized values > >>> > >>> Changes in v4: > >>> - fixed width calculation for zero-width, nonprintable and combined > >>> character. (pulled some code from linux-util) > >>> - added tests for cases mentioned above > >>> - changed usage of vshControl variables. From now on PrintToStdout calls > >>> PrintToString and then prints returned string to stdout > >>> > >>> Changes in v3: > >>> - changed encoding of 3/3 patch, otherwise it cannot be applied > >>> > >>> Changes in v2: > >>> - added tests > >>> - fixed alignment for unicode character which span more spaces > >>> - moved ncolumns check to vshTableRowAppend > >>> - changed arguments for functions vshTablePrint, vshTablePrintToStdout, > >>> vshTablePrintToString > >>> > >>> Simon Kobyda (3): > >>> vsh: Add API for printing tables. > >>> virsh: Implement new table API for virsh list > >>> vsh: Added tests > >>> > >>> tests/Makefile.am| 8 + > >>> tests/virshtest.c| 14 +- > >>> tests/vshtabletest.c | 377 + > >>> tools/Makefile.am| 4 +- > >>> tools/virsh-domain-monitor.c | 43 ++-- > >>> tools/vsh-table.c| 449 +++ > >>> tools/vsh-table.h| 42 > >>> 7 files changed, 910 insertions(+), 27 deletions(-) > >>> create mode 100644 tests/vshtabletest.c > >>> create mode 100644 tools/vsh-table.c > >>> create mode 100644 tools/vsh-table.h > >>> > >> > >> ACKed and pushed. > >> > >> Now we can start converting the rest of the code to use vshTable APIs. > > > > But first fix the build failures :-) > > > > On CentOS / RHEL: > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024141 > > > > > > 4) testUnicode ... > > Offset 30 > > Expect [государство > > - > > 1fedora28 running > > 2🙊🙉🙈rhel7.5🙆🙆🙅] > > Actual [ > > государство > > - > > 1fedora28 > > running > > 2 > > \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff] > > > > Okay, this is probably due to ancient gcc that's there (4.8.0) and is > supposed to be fixed by adding -finput-charset= onto gcc command line. > Haven't tested it though. > > > > > > > On Mingw: > > > > https://travis-ci.org/libvirt/libvirt/jobs/420024142 > > > > vsh-table.c: In function 'vshTableSafeEncode': > > vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' > > [-Werror=implicit-function-declaration] > > *width += wcwidth(wc); > >^~~ > > vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' > > [-Werror=nested-externs] > > But this is weird. wcwidth() manpage says to include wchar.t which we > are doing. Maybe _XOPEN_SOURCE macro is not defined? It is quite simple - the function doesn't exist on mingw :-) $ grep -r wcwidth /usr/i686-w64-mingw32/sys-root/mingw/include/ ...nothing... Looks like gnulib can rescue us though https://www.gnu.org/software/gnulib/manual/html_node/wcwidth.html Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] access: Fix nwfilter-binding ACL access API name generation
On Tue, Aug 21, 2018 at 04:23:25PM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1611320
>
> Generation of the ACL API policy is a "automated process"
> based on this perl script which "worked" with the changes to
> add nwfilter binding API's because they had the "nwfilter"
> prefix; however, the generated output name was incorrect
> based on the remote protocol algorithm which expected to
> generate names such as 'nwfilter-binding.action' instead
> of 'nwfilter.binding-action'.
>
> This effectively changes src/access/org.libvirt.api.policy entries:
>
> org.libvirt.api.nwfilter.binding-create ==>
> org.libvirt.api.nwfilter-binding.create
>
> org.libvirt.api.nwfilter.binding-delete ==>
> org.libvirt.api.nwfilter-binding.delete
>
> org.libvirt.api.nwfilter.binding-getattr ==>
> org.libvirt.api.nwfilter-binding.getattr
>
> org.libvirt.api.nwfilter.binding-read ==>
> org.libvirt.api.nwfilter-binding.read
>
> Signed-off-by: John Ferlan
Reviewed-by: Daniel P. Berrangé
> ---
>
> If someone can explain better exactly what is happening in this
> processing, I'd be more than willing to update the commit message.
> I'm sure my wording isn't "precise" enough, but I feel like I hit
> the lottery finding this needle in the haystack.
As you say, it is simply bad luck because the new APIs happened to
match the existing "nwfilter" prefix, so we didn't see the error
>
> src/access/genpolkit.pl | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/access/genpolkit.pl b/src/access/genpolkit.pl
> index 968cb8c55c..e074c90eb6 100755
> --- a/src/access/genpolkit.pl
> +++ b/src/access/genpolkit.pl
> @@ -22,8 +22,8 @@ use warnings;
>
> my @objects = (
> "CONNECT", "DOMAIN", "INTERFACE",
> -"NETWORK","NODE_DEVICE", "NWFILTER",
> - "SECRET", "STORAGE_POOL", "STORAGE_VOL",
> +"NETWORK","NODE_DEVICE", "NWFILTER_BINDING", "NWFILTER",
> +"SECRET", "STORAGE_POOL", "STORAGE_VOL",
> );
>
> my $objects = join ("|", @objects);
> --
> 2.17.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nwfilter: Handle libvirtd restart if nwfilter binding deleted
On Thu, Aug 23, 2018 at 07:59:41AM -0400, John Ferlan wrote:
>
>
> On 08/23/2018 07:27 AM, Daniel P. Berrangé wrote:
> > On Wed, Aug 22, 2018 at 05:43:21PM -0400, John Ferlan wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1607202
> >>
> >> It's stated that if the admin wants to shoot themselves in
> >> the foot by removing the nwfilter binding while the domain
> >
> > So based on your explanation in the other reply, this message
> > is what was misleading me. s/nwfilter binding/nwfilter/
> >
>
> Actually perhaps it's more by "first removing the nwfilter binding and
> subsequently undefining the nwfilter that is/was in use for the running
> guest..."
>
> If just the nwfilter binding was removed, then libvirtd restart would
> have been fine because it would have recreated the nwfilter binding. Of
> course that may not be expected either...
>
> >> is running we will certainly allow that. However, in doing
> >> so we also run the risk that a libvirtd restart will cause
> >> the domain to be shutdown, which isn't a good thing.
> >>
> >> So add another boolean to virDomainConfNWFilterInstantiate
> >> which allows us to recover somewhat gracefully in the event
> >> the virNWFilterBindingCreateXML fails when we come from
> >> qemuProcessReconnect and we determine that the filter has
> >> been deleted. It was there at some point (it had to be), but
> >> if it's missing, then we don't want to cause the guest to
> >> stop running, so issue a warning and continue on.
> >>
> >> Signed-off-by: John Ferlan
> >> ---
> >> src/conf/domain_nwfilter.c | 33 -
> >> src/conf/domain_nwfilter.h | 3 ++-
> >> src/lxc/lxc_process.c | 3 ++-
> >> src/qemu/qemu_hotplug.c| 7 ---
> >> src/qemu/qemu_interface.c | 6 --
> >> src/qemu/qemu_process.c| 10 +++---
> >> src/uml/uml_conf.c | 3 ++-
> >> 7 files changed, 49 insertions(+), 16 deletions(-)
> >
> > [snip]
> >
> >> static int
> >> -qemuProcessFiltersInstantiate(virDomainDefPtr def, bool ignoreExists)
> >> +qemuProcessFiltersInstantiate(virDomainDefPtr def,
> >> + bool ignoreExists,
> >> + bool ignoreDeleted)
> >> {
> >> size_t i;
> >>
> >> for (i = 0; i < def->nnets; i++) {
> >> virDomainNetDefPtr net = def->nets[i];
> >> if ((net->filter) && (net->ifname)) {
> >> -if (virDomainConfNWFilterInstantiate(def->name, def->uuid,
> >> net, ignoreExists) < 0)
> >> +if (virDomainConfNWFilterInstantiate(def->name, def->uuid,
> >> net,
> >> + ignoreExists,
> >> + ignoreDeleted) < 0)
> >> return 1;
> >> }
> >
> > Rather than this extra "ignoreDeleted" arg, why can't we just do
> >
> >if (virDomainConfNWFilterInstantiate(def->name, def->uuid, net,
> > ignoreExists) < 0 &&
> > ignoreDeleted)
> > return 1;
> >
> > This ensures that all things which can cause a nwfilter binding failure
> > on startup will be handled by avoiding tearing down the running guest.
> >
>
> Did you mean !ignoreDeleted? There's only one caller to
Heh, yes.
> qemuProcessFiltersInstantiate which does:
>
> if (qemuProcessFiltersInstantiate(obj->def, true))
> goto error;
>
> Of course what's the purpose of distinguishing between ignoreExists and
> ignoreDeleted then? Essentially what you're indicating is we wouldn't
> care about any error because virDomainConfNWFilterInstantiate wouldn't
> be distinguishing between any error (because there's only one caller to
> qemuProcessFiltersInstantiate).
Oh thats a good point - I forgot ignoreExists was for the same reason.
>
> I could change the last argument to virDomainConfNWFilterInstantiate to
> be a flag instead of a bool so that if we have future errors we care to
> ignore we don't keep adding bool arguments, but that's just a
> implementation detail.
We've deal with 1 problem scenario (alredy existing binding) and now
adding a second (missing filter). Will someone find a third scenario
and then a fourth. The flags argument ends up effectively being a
bitmask of lines where we ignore errors.
I'm wondering is it *ever* valid to treat failure of this filter call
as a fatal problem and teardown an already running VM ? My gut says no.
So perhaps we remove the ignoreExists parameter too, and just make that
one caller simply ignore the errors on restarts.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libv
Re: [libvirt] [PATCH v5 0/3] vsh: Introduce new API for printing tables
On 08/24/2018 11:36 AM, Daniel P. Berrangé wrote: > On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote: >> On 08/23/2018 05:53 PM, Simon Kobyda wrote: >>> Created new API for priting tables, mainly to solve alignment problems. >>> Implemented these test to virsh list. In the future, API may be >>> everywhere in virsh and virt-admin. >>> Also wrote basic tests for the new API, and corrected tests in virshtest >>> which are influenced by implementation of the API in virsh list. >>> >>> Changes in v5: >>> - cleanup and merged code for calculating zero-width, nonprintable and >>> combined >>> character. >>> - replaced virBufferAddStr with virBufferAddChar in some places >>> - in tests moved code for setting correct locale >>> - fixed few leaks and unitialized values >>> >>> Changes in v4: >>> - fixed width calculation for zero-width, nonprintable and combined >>> character. (pulled some code from linux-util) >>> - added tests for cases mentioned above >>> - changed usage of vshControl variables. From now on PrintToStdout calls >>> PrintToString and then prints returned string to stdout >>> >>> Changes in v3: >>> - changed encoding of 3/3 patch, otherwise it cannot be applied >>> >>> Changes in v2: >>> - added tests >>> - fixed alignment for unicode character which span more spaces >>> - moved ncolumns check to vshTableRowAppend >>> - changed arguments for functions vshTablePrint, vshTablePrintToStdout, >>> vshTablePrintToString >>> >>> Simon Kobyda (3): >>> vsh: Add API for printing tables. >>> virsh: Implement new table API for virsh list >>> vsh: Added tests >>> >>> tests/Makefile.am| 8 + >>> tests/virshtest.c| 14 +- >>> tests/vshtabletest.c | 377 + >>> tools/Makefile.am| 4 +- >>> tools/virsh-domain-monitor.c | 43 ++-- >>> tools/vsh-table.c| 449 +++ >>> tools/vsh-table.h| 42 >>> 7 files changed, 910 insertions(+), 27 deletions(-) >>> create mode 100644 tests/vshtabletest.c >>> create mode 100644 tools/vsh-table.c >>> create mode 100644 tools/vsh-table.h >>> >> >> ACKed and pushed. >> >> Now we can start converting the rest of the code to use vshTable APIs. > > But first fix the build failures :-) > > On CentOS / RHEL: > > https://travis-ci.org/libvirt/libvirt/jobs/420024141 > > > 4) testUnicode ... > Offset 30 > Expect [государство > - > 1fedora28 running > 2🙊🙉🙈rhel7.5🙆🙆🙅] > Actual [ > государство > - > 1fedora28 > running > 2 > \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff] > Okay, this is probably due to ancient gcc that's there (4.8.0) and is supposed to be fixed by adding -finput-charset= onto gcc command line. Haven't tested it though. > > > On Mingw: > > https://travis-ci.org/libvirt/libvirt/jobs/420024142 > > vsh-table.c: In function 'vshTableSafeEncode': > vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' > [-Werror=implicit-function-declaration] > *width += wcwidth(wc); >^~~ > vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' > [-Werror=nested-externs] But this is weird. wcwidth() manpage says to include wchar.t which we are doing. Maybe _XOPEN_SOURCE macro is not defined? Anyway, I'll let Simon to figure this out O:-) Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
On Thu, Aug 09, 2018 at 05:50:11PM +0200, Ján Tomko wrote:
> On Thu, Aug 09, 2018 at 11:21:52AM +0200, Katerina Koukiou wrote:
> > This patch ensures that changes in attributes of interfaces will be emit
> > errors accept if they are missing from the XML.
> > Previously we were falsely reporting successfull updates, because some
>
> *successful
>
> > changed attributes got overwritten before the validity checks.
>
> The more I think about this 'feature' that allows you to omit parts of
> the changed XML, the weirder it gets.
>
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1599513
> >
> > Signed-off-by: Katerina Koukiou
> > ---
> > src/qemu/qemu_hotplug.c | 42 +
> > 1 file changed, 26 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > index 1488f0a7c2..76ab56a479 100644
> > --- a/src/qemu/qemu_hotplug.c
> > +++ b/src/qemu/qemu_hotplug.c
> > @@ -3445,23 +3445,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
> > goto cleanup;
> > }
> >
> > -/* info: if newdev->info is empty, fill it in from olddev,
> > - * otherwise verify that it matches - nothing is allowed to
> > - * change. (There is no helper function to do this, so
> > - * individually check the few feidls of virDomainDeviceInfo that
> > - * are relevant in this case).
> > +/* info: Nothing is allowed to change. First fill the missing
> > newdev->info
> > + * from olddev and then check for changes.
>
> Maybe the other way around (checking first - with respect to what was
> specified in the XML, then overwriting) might be cleaner, see below.
>
> > */
> > -if (!virDomainDeviceAddressIsValid(&newdev->info,
> > - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
> > &&
> > -virDomainDeviceInfoCopy(&newdev->info, &olddev->info) < 0) {
> > -goto cleanup;
> > -}
> > -if (!virPCIDeviceAddressEqual(&olddev->info.addr.pci,
> > - &newdev->info.addr.pci)) {
> > -virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> > - _("cannot modify network device guest PCI
> > address"));
> > -goto cleanup;
> > -}
> > /* grab alias from olddev if not set in newdev */
> > if (!newdev->info.alias &&
> > VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0)
> > @@ -3469,26 +3455,50 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
> >
> > /* device alias is checked already in virDomainDefCompatibleDevice */
> >
> > +if (newdev->info.rombar == VIR_TRISTATE_BOOL_ABSENT)
> > +newdev->info.rombar = olddev->info.rombar;
> > if (olddev->info.rombar != newdev->info.rombar) {
> > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> >_("cannot modify network device rom bar setting"));
> > goto cleanup;
> > }
> > +
> > +if (!newdev->info.romfile &&
> > +VIR_STRDUP(newdev->info.romfile, olddev->info.romfile) < 0)
> > +goto cleanup;
> > if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
> > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> >_("cannot modify network rom file"));
> > goto cleanup;
> > }
> > +
> > +if (newdev->info.bootIndex == 0)
> > +newdev->info.bootIndex = olddev->info.bootIndex;
> > if (olddev->info.bootIndex != newdev->info.bootIndex) {
> > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> >_("cannot modify network device boot index
> > setting"));
> > goto cleanup;
> > }
> > +
> > +if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT)
> > +newdev->info.romenabled = olddev->info.romenabled;
> > if (olddev->info.romenabled != newdev->info.romenabled) {
> > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> >_("cannot modify network device rom enabled
> > setting"));
> > goto cleanup;
> > }
> > +
> > +/* if pci addr is missing or is invalid we overwrite it from olddev */
> > +if (!virDomainDeviceAddressIsValid(&newdev->info,
> > +
> > VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> > +newdev->info.addr.pci = olddev->info.addr.pci;
>
> This is an insufficient way to copy the address. The 'addr' union might
> be containing an address of a different type. And the 'info.type' attribute
> also
> needs to be copied, either 1) manually or via a 2) DeviceInfoCopy call.
I am going to add the type check before the addr check. Good point.
>
> Also, the checks can be moved to a separate fuction first (qemuDomainChangeNet
> is over 400 lines long now), e.g. qemuDomainChangeNetAllowed.
I am against splitting the checks and autofilling in different
functions, since it would be harder to spot if someone who is doing some
change changed both parts.
Katerina
Re: [libvirt] [PATCH v5 0/3] vsh: Introduce new API for printing tables
On Fri, Aug 24, 2018 at 10:59:04AM +0200, Michal Privoznik wrote: > On 08/23/2018 05:53 PM, Simon Kobyda wrote: > > Created new API for priting tables, mainly to solve alignment problems. > > Implemented these test to virsh list. In the future, API may be > > everywhere in virsh and virt-admin. > > Also wrote basic tests for the new API, and corrected tests in virshtest > > which are influenced by implementation of the API in virsh list. > > > > Changes in v5: > > - cleanup and merged code for calculating zero-width, nonprintable and > > combined > > character. > > - replaced virBufferAddStr with virBufferAddChar in some places > > - in tests moved code for setting correct locale > > - fixed few leaks and unitialized values > > > > Changes in v4: > > - fixed width calculation for zero-width, nonprintable and combined > > character. (pulled some code from linux-util) > > - added tests for cases mentioned above > > - changed usage of vshControl variables. From now on PrintToStdout calls > > PrintToString and then prints returned string to stdout > > > > Changes in v3: > > - changed encoding of 3/3 patch, otherwise it cannot be applied > > > > Changes in v2: > > - added tests > > - fixed alignment for unicode character which span more spaces > > - moved ncolumns check to vshTableRowAppend > > - changed arguments for functions vshTablePrint, vshTablePrintToStdout, > > vshTablePrintToString > > > > Simon Kobyda (3): > > vsh: Add API for printing tables. > > virsh: Implement new table API for virsh list > > vsh: Added tests > > > > tests/Makefile.am| 8 + > > tests/virshtest.c| 14 +- > > tests/vshtabletest.c | 377 + > > tools/Makefile.am| 4 +- > > tools/virsh-domain-monitor.c | 43 ++-- > > tools/vsh-table.c| 449 +++ > > tools/vsh-table.h| 42 > > 7 files changed, 910 insertions(+), 27 deletions(-) > > create mode 100644 tests/vshtabletest.c > > create mode 100644 tools/vsh-table.c > > create mode 100644 tools/vsh-table.h > > > > ACKed and pushed. > > Now we can start converting the rest of the code to use vshTable APIs. But first fix the build failures :-) On CentOS / RHEL: https://travis-ci.org/libvirt/libvirt/jobs/420024141 4) testUnicode ... Offset 30 Expect [государство - 1fedora28 running 2🙊🙉🙈rhel7.5🙆🙆🙅] Actual [ государство - 1fedora28 running 2 \xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xffrhel7.5\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff] On Mingw: https://travis-ci.org/libvirt/libvirt/jobs/420024142 vsh-table.c: In function 'vshTableSafeEncode': vsh-table.c:274:27: error: implicit declaration of function 'wcwidth' [-Werror=implicit-function-declaration] *width += wcwidth(wc); ^~~ vsh-table.c:274:27: error: nested extern declaration of 'wcwidth' [-Werror=nested-externs] Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/3] vsh: Add API for printing tables.
On 08/23/2018 05:53 PM, Simon Kobyda wrote:
> It solves problems with alignment of columns. Width of each column
> is calculated by its biggest cell. Should solve unicode bug.
> In future, it may be implemented in virsh, virt-admin...
>
> This API has 5 public functions:
> - vshTableNew - adds new table and defines its header
> - vshTableRowAppend - appends new row (for same number of columns as in
> header)
> - vshTablePrintToStdout
> - vshTablePrintToString
> - vshTableFree
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1574624
> https://bugzilla.redhat.com/show_bug.cgi?id=1584630
>
> Signed-off-by: Simon Kobyda
> ---
> tools/Makefile.am | 4 +-
> tools/vsh-table.c | 449 ++
> tools/vsh-table.h | 42 +
> 3 files changed, 494 insertions(+), 1 deletion(-)
> create mode 100644 tools/vsh-table.c
> create mode 100644 tools/vsh-table.h
>
> diff --git a/tools/vsh-table.c b/tools/vsh-table.c
> new file mode 100644
> index 00..ca4e9265c5
> --- /dev/null
> +++ b/tools/vsh-table.c
> +/**
> + * Function pulled from util-linux
> + *
> + * Function's name in util-linux: mbs_safe_encode_to_buffer
> + *
> + * Returns allocated string where all control and non-printable chars are
> + * replaced with \x?? hex sequence, or NULL.
> + */
> +static char *
> +vshTableSafeEncode(const char *s, size_t *width)
> +{
> +const char *p = s;
> +size_t sz = s ? strlen(s) : 0;
> +char *buf;
> +char *ret;
> +mbstate_t st;
> +
> +memset(&st, 0, sizeof(st));
> +
> +if (VIR_ALLOC_N(buf, (sz * HEX_ENCODE_LENGTH) + 1) < 0)
> +return NULL;
> +
> +if (!sz)
> +return NULL;
You need to swap these two lines otherwise @buf is leaked.
> +
> +ret = buf;
> +*width = 0;
> +
> +while (p && *p) {
> +if ((*p == '\\' && *(p + 1) == 'x') ||
> +c_iscntrl(*p)) {
> +snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", *p);
> +buf += HEX_ENCODE_LENGTH;
> +*width += HEX_ENCODE_LENGTH;
> +p++;
> +} else {
> +wchar_t wc;
> +size_t len = mbrtowc(&wc, p, MB_CUR_MAX, &st);
> +
> +if (len == 0)
> +break; /* end of string */
> +
> +if (len == (size_t) -1 || len == (size_t) -2) {
> +len = 1;
> +/*
> + * Not valid multibyte sequence -- maybe it's
> + * printable char according to the current locales.
> + */
> +if (!c_isprint(*p)) {
> +snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", *p);
> +buf += HEX_ENCODE_LENGTH;
> +*width += HEX_ENCODE_LENGTH;
> +} else {
> +(*width)++;
> +*buf++ = *p;
> +}
> +} else if (!iswprint(wc)) {
> +size_t i;
> +for (i = 0; i < len; i++) {
> +snprintf(buf, HEX_ENCODE_LENGTH + 1, "\\x%02x", p[i]);
> +buf += HEX_ENCODE_LENGTH;
> +*width += HEX_ENCODE_LENGTH;
> +}
> +} else {
> +memcpy(buf, p, len);
> +buf += len;
> +*width += wcwidth(wc);
> +}
> +p += len;
> +}
> +}
> +
> +*buf = '\0';
> +return ret;
> +}
> +
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 0/3] vsh: Introduce new API for printing tables
On 08/23/2018 05:53 PM, Simon Kobyda wrote: > Created new API for priting tables, mainly to solve alignment problems. > Implemented these test to virsh list. In the future, API may be > everywhere in virsh and virt-admin. > Also wrote basic tests for the new API, and corrected tests in virshtest > which are influenced by implementation of the API in virsh list. > > Changes in v5: > - cleanup and merged code for calculating zero-width, nonprintable and > combined > character. > - replaced virBufferAddStr with virBufferAddChar in some places > - in tests moved code for setting correct locale > - fixed few leaks and unitialized values > > Changes in v4: > - fixed width calculation for zero-width, nonprintable and combined > character. (pulled some code from linux-util) > - added tests for cases mentioned above > - changed usage of vshControl variables. From now on PrintToStdout calls > PrintToString and then prints returned string to stdout > > Changes in v3: > - changed encoding of 3/3 patch, otherwise it cannot be applied > > Changes in v2: > - added tests > - fixed alignment for unicode character which span more spaces > - moved ncolumns check to vshTableRowAppend > - changed arguments for functions vshTablePrint, vshTablePrintToStdout, > vshTablePrintToString > > Simon Kobyda (3): > vsh: Add API for printing tables. > virsh: Implement new table API for virsh list > vsh: Added tests > > tests/Makefile.am| 8 + > tests/virshtest.c| 14 +- > tests/vshtabletest.c | 377 + > tools/Makefile.am| 4 +- > tools/virsh-domain-monitor.c | 43 ++-- > tools/vsh-table.c| 449 +++ > tools/vsh-table.h| 42 > 7 files changed, 910 insertions(+), 27 deletions(-) > create mode 100644 tests/vshtabletest.c > create mode 100644 tools/vsh-table.c > create mode 100644 tools/vsh-table.h > ACKed and pushed. Now we can start converting the rest of the code to use vshTable APIs. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/9] util: add RISC-V architectures
On Thu, 2018-08-23 at 17:30 +0200, Andrea Bolognani wrote: > On Wed, 2018-08-22 at 11:15 +0200, Lubomir Rintel wrote: > > +VIR_ARCH_RISCV32, /* RISC-V 64 LE > > http://en.wikipedia.org/wiki/RISC-V */ > > This should be 32 LE ... > > > +VIR_ARCH_RISCV64, /* RISC-V 32 BE > > http://en.wikipedia.org/wiki/RISC-V */ > > ... and this should be 64 LE > > according to the corresponding implementation. > > I'll take care of it before pushing. I also had to squash in the diff below, which is necessary for the build to succeed after commit d1a6d73ddff2 (pushed minutes ago, so your series couldn't possibly account for that :). diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6a3914bde2..9fae713c63 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2298,15 +2298,18 @@ static const char *preferredMachines[] = "pseries", /* VIR_ARCH_PPC64LE */ "bamboo", /* VIR_ARCH_PPCEMB */ +"spike_v1.10", /* VIR_ARCH_RISCV32 */ +"spike_v1.10", /* VIR_ARCH_RISCV64 */ NULL, /* VIR_ARCH_S390 (no QEMU impl) */ "s390-ccw-virtio", /* VIR_ARCH_S390X */ "shix", /* VIR_ARCH_SH4 */ + "shix", /* VIR_ARCH_SH4EB */ "SS-5", /* VIR_ARCH_SPARC */ - "sun4u", /* VIR_ARCH_SPARC64 */ "puv3", /* VIR_ARCH_UNICORE32 */ "pc", /* VIR_ARCH_X86_64 */ + "sim", /* VIR_ARCH_XTENSA */ "sim", /* VIR_ARCH_XTENSAEB */ }; -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 00/12] lcitool: Add 'build' action
On Wed, Aug 22, 2018 at 11:44:15AM +0200, Andrea Bolognani wrote: > Changes from [v2]: > > * rebase on top of master (dbc2de85f775) and integrate recent > changes to build rules on the Jenkins side; > > * drop a commit that had already been merged in the meantime. > > Changes from [v1]: > > * rebase on top of master (985ab833be9b) and integrate recent > changes to build rules on the Jenkins side; > > * build on more targets. > > [v2] https://www.redhat.com/archives/libvir-list/2018-August/msg01109.html > [v1] https://www.redhat.com/archives/libvir-list/2018-August/msg00393.html Except for the arbitrary branch building, I think we're good and this can go in. Thank you for the feature. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 07/12] guests: Add build playbook
On Wed, Aug 22, 2018 at 11:44:22AM +0200, Andrea Bolognani wrote: > This playbook represent the entry point for automated > builds, and follows the same calling conventions as the > existing update playbook. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 06/12] guests: Add build projects
On Wed, Aug 22, 2018 at 11:44:21AM +0200, Andrea Bolognani wrote: > These tasks mirror the Jenkins projects contained in the > top-level projects/ directory. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v3 05/12] guests: Add build jobs
On Wed, Aug 22, 2018 at 11:44:20AM +0200, Andrea Bolognani wrote: > These tasks mirror the Jenkins jobs contained in the > top-level jobs/ directory. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 2/3] ui: increase min required GTK3 version to 3.14.0
From: Daniel P. Berrangé
Per supported platforms doc[1], the various min GTK3 on relevant distros is:
RHEL-7.0: 3.8.8
RHEL-7.2: 3.14.13
RHEL-7.4: 3.22.10
RHEL-7.5: 3.22.26
Debian (Stretch): 3.22.11
Debian (Jessie): 3.14.5
OpenBSD (Ports): 3.22.30
FreeBSD (Ports): 3.22.29
OpenSUSE Leap 15: 3.22.30
SLE12-SP2: Unknown
Ubuntu (Xenial): 3.18.9
macOS (Homebrew): 3.22.30
This suggests that a minimum GTK3 of 3.14.0 is a reasonable target,
as users are unlikely to be stuck on RHEL-7.0/7.1 still
[1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
Signed-off-by: Daniel P. Berrangé
Message-id: 20180822131554.3398-3-berra...@redhat.com
Signed-off-by: Gerd Hoffmann
---
configure | 2 +-
ui/gtk.c | 10 --
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/configure b/configure
index f982d8bd61..eca30885d6 100755
--- a/configure
+++ b/configure
@@ -2646,7 +2646,7 @@ fi
if test "$gtk" != "no"; then
gtkpackage="gtk+-3.0"
gtkx11package="gtk+-x11-3.0"
-gtkversion="3.0.0"
+gtkversion="3.14.0"
if $pkg_config --exists "$gtkpackage >= $gtkversion"; then
gtk_cflags=$($pkg_config --cflags $gtkpackage)
gtk_libs=$($pkg_config --libs $gtkpackage)
diff --git a/ui/gtk.c b/ui/gtk.c
index 9d09bc9e57..6d57558084 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -998,7 +998,6 @@ static gboolean gd_scroll_event(GtkWidget *widget,
GdkEventScroll *scroll,
btn = INPUT_BUTTON_WHEEL_UP;
} else if (scroll->direction == GDK_SCROLL_DOWN) {
btn = INPUT_BUTTON_WHEEL_DOWN;
-#if GTK_CHECK_VERSION(3, 4, 0)
} else if (scroll->direction == GDK_SCROLL_SMOOTH) {
gdouble delta_x, delta_y;
if (!gdk_event_get_scroll_deltas((GdkEvent *)scroll,
@@ -1010,7 +1009,6 @@ static gboolean gd_scroll_event(GtkWidget *widget,
GdkEventScroll *scroll,
} else {
btn = INPUT_BUTTON_WHEEL_UP;
}
-#endif
} else {
return TRUE;
}
@@ -1672,11 +1670,9 @@ static GSList *gd_vc_menu_init(GtkDisplayState *s,
VirtualConsole *vc,
gtk_accel_group_connect(s->accel_group, GDK_KEY_1 + idx,
HOTKEY_MODIFIERS, 0,
g_cclosure_new_swap(G_CALLBACK(gd_accel_switch_vc), vc, NULL));
-#if GTK_CHECK_VERSION(3, 8, 0)
gtk_accel_label_set_accel(
GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(vc->menu_item))),
GDK_KEY_1 + idx, HOTKEY_MODIFIERS);
-#endif
g_signal_connect(vc->menu_item, "activate",
G_CALLBACK(gd_menu_switch_vc), s);
@@ -2167,11 +2163,9 @@ static GtkWidget *gd_create_menu_view(GtkDisplayState *s)
TRUE);
gtk_accel_group_connect(s->accel_group, GDK_KEY_m, HOTKEY_MODIFIERS, 0,
g_cclosure_new_swap(G_CALLBACK(gd_accel_show_menubar), s, NULL));
-#if GTK_CHECK_VERSION(3, 8, 0)
gtk_accel_label_set_accel(
GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(s->show_menubar_item))),
GDK_KEY_m, HOTKEY_MODIFIERS);
-#endif
gtk_menu_shell_append(GTK_MENU_SHELL(view_menu), s->show_menubar_item);
return view_menu;
@@ -2221,11 +2215,7 @@ static void gtk_display_init(DisplayState *ds,
DisplayOptions *opts)
s->opts = opts;
s->window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
-#if GTK_CHECK_VERSION(3, 2, 0)
s->vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0);
-#else
-s->vbox = gtk_vbox_new(FALSE, 0);
-#endif
s->notebook = gtk_notebook_new();
s->menu_bar = gtk_menu_bar_new();
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 1/3] ui: remove support for GTK2 in favour of GTK3
From: Daniel P. Berrangé GTK2 was deprecated in the 2.12.0 release with: commit b7715af2b31f47060cc5b4be930d16c13be93fa9 Author: Daniel P. Berrange Date: Tue Dec 12 11:34:40 2017 + ui: deprecate use of GTK 2.x in favour of 3.x series The GTK 3.0 release was made in Feb, 2011: https://blog.gtk.org/2011/02/10/gtk-3-0-released/ That will soon be 7 years ago, which is enough time to consider the 3.x series widely supported. Thus we deprecate the GTK 2.x support, which will allow us to delete it in the last release of 2018. By this time, GTK 3.x will be almost 8 years old. Signed-off-by: Daniel P. Berrange Message-id: 20171212113440.16483-1-berra...@redhat.com Signed-off-by: Gerd Hoffmann It is thus able to be removed in the 3.1.0 release. Signed-off-by: Daniel P. Berrangé Message-id: 20180822131554.3398-2-berra...@redhat.com Signed-off-by: Gerd Hoffmann --- configure| 51 +++--- include/ui/gtk.h | 9 --- ui/gtk-egl.c | 10 ++- ui/gtk.c | 192 --- qemu-deprecated.texi | 7 -- 5 files changed, 26 insertions(+), 243 deletions(-) diff --git a/configure b/configure index e7bddc04b0..f982d8bd61 100755 --- a/configure +++ b/configure @@ -453,7 +453,6 @@ glusterfs_discard="no" glusterfs_fallocate="no" glusterfs_zerofill="no" gtk="" -gtkabi="" gtk_gl="no" tls_priority="NORMAL" gnutls="" @@ -1369,8 +1368,6 @@ for opt do ;; --disable-pvrdma) pvrdma="no" ;; - --with-gtkabi=*) gtkabi="$optarg" - ;; --disable-vte) vte="no" ;; --enable-vte) vte="yes" @@ -1658,7 +1655,6 @@ disabled with --disable-FEATURE, default is enabled if available: sdl SDL UI --with-sdlabi select preferred SDL ABI 1.2 or 2.0 gtk gtk UI - --with-gtkabi select preferred GTK ABI 2.0 or 3.0 vte vte support for the gtk UI curses curses UI vnc VNC UI support @@ -2648,24 +2644,9 @@ fi # GTK probe if test "$gtk" != "no"; then -if test "$gtkabi" = ""; then -# The GTK ABI was not specified explicitly, so try whether 3.0 is available. -# Use 2.0 as a fallback if that is available. -if $pkg_config --exists "gtk+-3.0 >= 3.0.0"; then -gtkabi=3.0 -elif $pkg_config --exists "gtk+-2.0 >= 2.18.0"; then -gtkabi=2.0 -else -gtkabi=3.0 -fi -fi -gtkpackage="gtk+-$gtkabi" -gtkx11package="gtk+-x11-$gtkabi" -if test "$gtkabi" = "3.0" ; then - gtkversion="3.0.0" -else - gtkversion="2.18.0" -fi +gtkpackage="gtk+-3.0" +gtkx11package="gtk+-x11-3.0" +gtkversion="3.0.0" if $pkg_config --exists "$gtkpackage >= $gtkversion"; then gtk_cflags=$($pkg_config --cflags $gtkpackage) gtk_libs=$($pkg_config --libs $gtkpackage) @@ -2909,16 +2890,11 @@ fi # VTE probe if test "$vte" != "no"; then -if test "$gtkabi" = "3.0"; then - vteminversion="0.32.0" - if $pkg_config --exists "vte-2.91"; then -vtepackage="vte-2.91" - else -vtepackage="vte-2.90" - fi +vteminversion="0.32.0" +if $pkg_config --exists "vte-2.91"; then + vtepackage="vte-2.91" else - vtepackage="vte" - vteminversion="0.24.0" + vtepackage="vte-2.90" fi if $pkg_config --exists "$vtepackage >= $vteminversion"; then vte_cflags=$($pkg_config --cflags $vtepackage) @@ -2926,11 +2902,7 @@ if test "$vte" != "no"; then vteversion=$($pkg_config --modversion $vtepackage) vte="yes" elif test "$vte" = "yes"; then -if test "$gtkabi" = "3.0"; then -feature_not_found "vte" "Install libvte-2.90/2.91 devel" -else -feature_not_found "vte" "Install libvte devel" -fi +feature_not_found "vte" "Install libvte-2.90/2.91 devel" else vte="no" fi @@ -6089,12 +6061,6 @@ if test "$sdl_too_old" = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" fi -if test "$gtkabi" = "2.0"; then -echo -echo "WARNING: Use of GTK 2.0 is deprecated and will be removed in" -echo "WARNING: future releases. Please switch to using GTK 3.0" -fi - if test "$sdlabi" = "1.2"; then echo echo "WARNING: Use of SDL 1.2 is deprecated and will be removed in" @@ -6414,7 +6380,6 @@ if test "$glib_subprocess" = "yes" ; then fi if test "$gtk" = "yes" ; then echo "CONFIG_GTK=m" >> $config_host_mak - echo "CONFIG_GTKABI=$gtkabi" >> $config_host_mak echo "GTK_CFLAGS=$gtk_cflags" >> $config_host_mak echo "GTK_LIBS=$gtk_libs" >> $config_host_mak if test "$gtk_gl" = "yes" ; then diff --git a/include/ui/gtk.h b/include/ui/gtk.h index a79780afc7..99edd3c085 100644 --- a/include/ui/gtk.h +++ b/include/ui/gtk.h @@ -27,15 +27,6 @@ #include "ui/egl-context.h" #endif -/* Compatibility define to let us b
[libvirt] [PULL 3/3] ui: remove support for SDL1.2 in favour of SDL2
From: Daniel P. Berrangé
SDL1.2 was deprecated in the 2.12.0 release with:
commit e52c6ba34149b4f39c3fd60e59ee32b809db2bfa
Author: Daniel P. Berrange
Date: Mon Jan 15 14:25:33 2018 +
ui: deprecate use of SDL 1.2 in favour of 2.0 series
The SDL 2.0 release was made in Aug, 2013:
https://www.libsdl.org/release/
That will soon be 4 + 1/2 years ago, which is enough time to consider
the 2.0 series widely supported.
Thus we deprecate the SDL 1.2 support, which will allow us to delete it
in the last release of 2018. By this time, SDL 2.0 will be more than 5
years old.
Signed-off-by: Daniel P. Berrange
Reviewed-by: Marc-André Lureau
Message-id: 20180115142533.24585-1-berra...@redhat.com
Signed-off-by: Gerd Hoffmann
It is thus able to be removed in the 3.1.0 release.
Signed-off-by: Daniel P. Berrangé
Message-id: 20180822131554.3398-4-berra...@redhat.com
Signed-off-by: Gerd Hoffmann
---
configure | 60 +--
ui/sdl_zoom.h | 25 --
ui/sdl_zoom_template.h | 219 ---
ui/sdl.c | 1027
ui/sdl_zoom.c | 93 -
qemu-deprecated.texi |9 -
ui/Makefile.objs |5 -
7 files changed, 7 insertions(+), 1431 deletions(-)
delete mode 100644 ui/sdl_zoom.h
delete mode 100644 ui/sdl_zoom_template.h
delete mode 100644 ui/sdl.c
delete mode 100644 ui/sdl_zoom.c
diff --git a/configure b/configure
index eca30885d6..c7728e9355 100755
--- a/configure
+++ b/configure
@@ -344,7 +344,6 @@ docs=""
fdt=""
netmap="no"
sdl=""
-sdlabi=""
virtfs=""
mpath=""
vnc="yes"
@@ -565,7 +564,6 @@ query_pkg_config() {
"${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
}
pkg_config=query_pkg_config
-sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
# If the user hasn't specified ARFLAGS, default to 'rv', just as make does.
@@ -1028,8 +1026,6 @@ for opt do
;;
--enable-sdl) sdl="yes"
;;
- --with-sdlabi=*) sdlabi="$optarg"
- ;;
--disable-qom-cast-debug) qom_cast_debug="no"
;;
--enable-qom-cast-debug) qom_cast_debug="yes"
@@ -1653,7 +1649,6 @@ disabled with --disable-FEATURE, default is enabled if
available:
nettle nettle cryptography support
gcrypt libgcrypt cryptography support
sdl SDL UI
- --with-sdlabi select preferred SDL ABI 1.2 or 2.0
gtk gtk UI
vte vte support for the gtk UI
curses curses UI
@@ -2916,37 +2911,11 @@ fi
sdl_probe ()
{
- sdl_too_old=no
- if test "$sdlabi" = ""; then
- if $pkg_config --exists "sdl2"; then
- sdlabi=2.0
- elif $pkg_config --exists "sdl"; then
- sdlabi=1.2
- else
- sdlabi=2.0
- fi
- fi
-
- if test $sdlabi = "2.0"; then
- sdl_config=$sdl2_config
- sdlname=sdl2
- sdlconfigname=sdl2_config
- elif test $sdlabi = "1.2"; then
- sdlname=sdl
- sdlconfigname=sdl_config
- else
- error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
- fi
-
- if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config};
then
-sdl_config=$sdlconfigname
- fi
-
- if $pkg_config $sdlname --exists; then
-sdlconfig="$pkg_config $sdlname"
+ if $pkg_config sdl2 --exists; then
+sdlconfig="$pkg_config sdl2"
sdlversion=$($sdlconfig --modversion 2>/dev/null)
elif has ${sdl_config}; then
-sdlconfig="$sdl_config"
+sdlconfig="$sdl2_config"
sdlversion=$($sdlconfig --version)
else
if test "$sdl" = "yes" ; then
@@ -2968,8 +2937,8 @@ EOF
sdl_cflags=$($sdlconfig --cflags 2>/dev/null)
sdl_cflags="$sdl_cflags -Wno-undef" # workaround 2.0.8 bug
if test "$static" = "yes" ; then
-if $pkg_config $sdlname --exists; then
- sdl_libs=$($pkg_config $sdlname --static --libs 2>/dev/null)
+if $pkg_config sdl2 --exists; then
+ sdl_libs=$($pkg_config sdl2 --static --libs 2>/dev/null)
else
sdl_libs=$($sdlconfig --static-libs 2>/dev/null)
fi
@@ -2977,11 +2946,7 @@ EOF
sdl_libs=$($sdlconfig --libs 2>/dev/null)
fi
if compile_prog "$sdl_cflags" "$sdl_libs" ; then
-if test $(echo $sdlversion | sed 's/[^0-9]//g') -lt 121 ; then
- sdl_too_old=yes
-else
- sdl=yes
-fi
+sdl=yes
# static link with sdl ? (note: sdl.pc's --static --libs is broken)
if test "$sdl" = "yes" -a "$static" = "yes" ; then
@@ -2997,7 +2962,7 @@ EOF
fi # static link
else # sdl not found
if test "$sdl" = "yes" ; then
- feature_not_found "sdl" "Install SDL devel"
+ feature_not_found "sdl" "Install SDL2 devel"
fi
sdl=no
fi # sdl compile test
@@ -6057,16 +6022,6 @@ echo "capstone $capstone"
echo "docker$docker"
echo "libpmem support $libpmem"
-if test "$sdl_too_old" = "yes"; then
-echo "-> Your SDL version is too old - please upgrade to have SDL support"
-
[libvirt] [PULL 0/3] Ui2 20180824 patches
The following changes since commit 5ccac548faf041ff5229a8e8342e3be14a34c8af: Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2018-08-23 17:35:48 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/ui2-20180824-pull-request for you to fetch changes up to eb01505ea6c8330273d189acc053cc60d00bfa1a: ui: remove support for SDL1.2 in favour of SDL2 (2018-08-24 09:31:39 +0200) ui: remove deprecated UI frontends Daniel P. Berrangé (3): ui: remove support for GTK2 in favour of GTK3 ui: increase min required GTK3 version to 3.14.0 ui: remove support for SDL1.2 in favour of SDL2 configure | 111 +- include/ui/gtk.h |9 - ui/sdl_zoom.h | 25 -- ui/sdl_zoom_template.h | 219 --- ui/gtk-egl.c | 10 +- ui/gtk.c | 202 +- ui/sdl.c | 1027 ui/sdl_zoom.c | 93 - qemu-deprecated.texi | 16 - ui/Makefile.objs |5 - 10 files changed, 33 insertions(+), 1684 deletions(-) delete mode 100644 ui/sdl_zoom.h delete mode 100644 ui/sdl_zoom_template.h delete mode 100644 ui/sdl.c delete mode 100644 ui/sdl_zoom.c -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] apparmor: fix ptrace rules with kernel 4.18
On Fri, Aug 24, 2018 at 08:12:11AM +0200, Christian Ehrhardt wrote:
> Due to kernel upstream change 338d0be4 ("apparmor: fix ptrace read check")
> libvirt now hits apparmor denies like:
> apparmor="DENIED" operation="ptrace" profile="/usr/sbin/libvirtd"
> pid=4409 comm="libvirtd" requested_mask="read" denied_mask="read"
> peer="libvirt-14e92a75-7668-4b97-8f92-322fc1b9c78a"
>
> Extend the ptrace rule to also allow 'ptrace (read)' for libvirtd to work
> with these newer kernels.
>
> Fixes: https://bugs.launchpad.net/bugs/1788603
>
> Reported-by: Thadeu Lima de Souza Cascardo
> Signed-off-by: Christian Ehrhardt
> ---
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Make sure preferredMachines is not missing any entry
On Thu, Aug 23, 2018 at 06:57:24PM +0200, Andrea Bolognani wrote: > With the current implementation, adding a new architecture > and not updating preferredMachines accordingly will not > cause a build failure, making it very likely that subtle > bugs will be introduced in the process. Rework the code > so that such issues will be caught by the compiler. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 08/12] conf: Allocate/release 'uid' and 'fid' in PCI address
在 2018/8/23 下午7:01, Andrea Bolognani 写道:
On Thu, 2018-08-23 at 18:01 +0800, Yi Min Zhao wrote:
在 2018/8/22 下午5:56, Andrea Bolognani 写道:
The patches I said I'd write are now on the list:
https://www.redhat.com/archives/libvir-list/2018-August/msg01105.html
No reviews yet, we'll see whether they get ACKed or NACKed :)
I saw them. Could I give my ACKed?
Feel free to do that, but before pushing I'd like to have Laine's
ACK since he's the one who introduced the functions in the first
place.
I have a question about your previous comment about error report.
You thought we should report more specific information.
+virZPCIDeviceAddressPtr zpci = dev->addr.pci.zpci;
+
+if (zpci && !dev->pciAddressExtFlags) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("zPCI is not
supported."));
+return -1;
+}
It's called by all device types which possibly use PCI address.
I'm not sure how to report device's name in error string.
I think reporting the address is enough. The idea is to give the
user some information they can use to figure out which device is
misconfigured rather than just telling them there is an issue and
leaving it at that.
I also think we might be at a point where enough big changes have
been made that the best way forward is probably to post a v3 that
includes those and then use that as the basis for more fine-tuning
for minor stuff such as error messages and the like. Does that
sound reasonable?
Yes, it makes sense.
I'm preparing V4 and could post it today.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
