Re: [libvirt] [PATCH python] Avoid comparing boolean and integers

2017-08-09 Thread Eric Blake 
On 08/09/2017 11:07 AM, Daniel P. Berrange wrote:
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Did you get a compiler that complained?

> 
> diff --git a/libvirt-utils.c b/libvirt-utils.c
> index 727397d..0af13dc 100644
> --- a/libvirt-utils.c
> +++ b/libvirt-utils.c
> @@ -108,7 +108,7 @@ virReallocN(void *ptrptr,
>  return -1;
>  }
>  tmp = realloc(*(void**)ptrptr, size * count);
> -if (!tmp && (size * count)) {
> +if (!tmp && ((size * count) != 0)) {

Do you need the extra (), or will one of these shorter forms also work?

if (!tmp && size * count != 0)
if (!tmp && (size * count) != 0)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RESEND] pci: add support for VMD domains

2017-08-09 Thread Jon Derrick 
Hi Laine,

Here's an alternate patch which is probably a lot less potentially
harmful. Please let me know what you think:


commit 931ac8a0a32c3399c92efd18e4478342aedec9e1
Author: Jon Derrick 
Date:   Wed Aug 9 13:10:01 2017 -0600

pci: Restrict devices to 16-bit domains

Most pci devices are limited to 16-bit domains, but VMD uses 32-bit
domains for child devices. In the current VMD model, child devices
cannot be assigned to VMs. So restrict the domains to 16-bits rather
than allowing virpci to attempt to enumerate the devices and failing
with the following error message:

internal error: dev->name buffer overflow: 1:00:00.0

Signed-off-by: Jon Derrick 

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 2c1b758..7aaaddd 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1769,6 +1769,12 @@ virPCIDeviceNew(unsigned int domain,
 dev->address.slot = slot;
 dev->address.function = function;

+/* This is already implied by the following snprintf, but restrict
devices
+ * to 16-bit domains to prevent the error message
+ */
+if (domain > 0x)
+return NULL;
+
 if (snprintf(dev->name, sizeof(dev->name), "%.4x:%.2x:%.2x.%.1x",
  domain, bus, slot, function) >= sizeof(dev->name)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,

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

[libvirt] Qemu only support VHDx disk format, but libvirt force use VHD format

2017-08-09 Thread Komeiji Kuroko 
Hello all,

I am trying to use a VHDx format disk on libvirt.

According to https://en.wikibooks.org/wiki/QEMU/Images, Qemu supports 
VHDx but not VHD.

But when I set disk format to 'vhdx' in virt-manager, it prompts "Error 
changing VM configuration: Expected a wrapped C Object but got "

Traceback (most recent call last):
   File "/usr/share/virt-manager/virtManager/addhardware.py", line
765, in change_config_helper
 define_func(devobj=devobj, do_hotplug=False, **define_args)
   File "/usr/share/virt-manager/virtManager/domain.py", line 847,
in define_disk
 self._redefine_xmlobj(xmlobj)
   File "/usr/share/virt-manager/virtManager/libvirtobject.py", line
389, in _redefine_xmlobj
 self._define(newxml)
   File "/usr/share/virt-manager/virtManager/domain.py", line 1156,
in _define
 self.conn.define_domain(newxml)
   File "/usr/share/virt-manager/virtManager/connection.py", line
678, in define_domain
 return self._backend.defineXML(xml)
   File "/usr/lib/python2.7/dist-packages/libvirt.py", line 3622, in
defineXML
 __tmp = virDomain(self,_obj=ret)
   File "/usr/lib/python2.7/dist-packages/libvirt.py", line 453, in
__init__
 raise Exception("Expected a wrapped C Object but got %s" %
type(_obj))
Exception: Expected a wrapped C Object but got 

Then I use `virsh edit MyDOM` to edit my domain XML, change disk format 
to "vhdx" then I get error:

"Unable to validate doc against /usr/share/libvirt/schemas/domain.rng".

So I edit /usr/share/libvirt/schemas/storagecommon.rng, change 
"vhd" to "vhdx" in  . But it still not works, and virsh tells me 
`error: unsupported configuration: unknown driver format value 'vhdx' `. 
I choice ignore , but then same error and I can't ignore: only Yes to 
re-edit or No to give up.

Then I suppose "vhd" in libvirt means "vhdx", so I change disk format to 
"vhd". But then when I run domain, Qemu get error: " Unknown driver 
'vhd' ". It seems libvirt pass "vhd" to Qemu, but Qemu only support "vhdx"

At last, I use Qemu paramaters: ` -drive 
file=/Path/To/File.vhdx,format=vhdx,if=none,id=drive-sata0-0-3 `, and it 
works. When I use `-drive format=vhd` , Qemu gives me same error: " 
Unknown driver 'vhd' ".

So I think it maybe a bug that libvirt forcely demand format is "vhd" 
and I find no way to change it, but in same time Qemu only accept format 
"vhdx" as paramater. I try to edit 
`/usr/lib/python2.7/dist-packages/libvirt.py` but I have no idea how to 
modify it. And it seems only changing of storagecommon.rng is not enough.

My libvirt version is 3.6.0 and Qemu version is 2.8.1.

Regards,

Mitori


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

Re: [libvirt] [RESEND] pci: add support for VMD domains

2017-08-09 Thread Jon Derrick 
Hi Laine,

Thanks for the thoughtful reply.

On 08/08/2017 05:35 PM, Laine Stump wrote:
> On 08/07/2017 03:46 PM, Jon Derrick wrote:
>> VMD domains start at 0x1, so expand dev->name to fit at least this > 
>> many characters. > > Signed-off-by: Jon Derrick
>  > --- > src/util/virpci.c | 2 +- > 1 file
> changed, 1 insertion(+), 1 deletion(-) > > diff --git
> a/src/util/virpci.c b/src/util/virpci.c > index 2c1b758..b3afefe 100644
>> --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -50,7 +50,7 @@
> VIR_LOG_INIT("util.pci"); > > #define PCI_SYSFS "/sys/bus/pci/" >
> #define PCI_ID_LEN 10 /* " " */ > -#define PCI_ADDR_LEN 13 /*
> ":XX:XX.X" */ > +#define PCI_ADDR_LEN 14 /* "X:XX:XX.X" */ > >
> VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST, > "", "2.5",
> "5", "8", "16")
> Does just this change by itself enable new functionality? Or are other
> changes required? (e.g. the type "pciDomain" in the XML schema is a
> uint16, so any domain > 0x in the config would fail validation).
It doesn't actually enable new functionality. The VMD product itself
doesn't currently support passthrough of child devices, so a 32-bit
domain will never be bound to a VM. This is being ensured by a kernel
patch effort[1], so this patch can wait until those land.

Onto other points, I see that pciDomain is uint16_t, however with my
patch I am able to return the following (which I'm not sure if it means
it still needs changing or not):

  pci_1_11_00_0

/sys/devices/pci:5d/:5d:05.5/pci1:00/1:00:00.0/1:01:00.0/1:02:07.0/1:06:00.0/1:07:10.0/1:11:00.0
  pci_1_07_10_0
  
nvme
  
  
65536
17
0
0
PCIe Data Center SSD
Intel Corporation


  
  

  




However the main goal of the patch was to squash the following warning
and potential issue:
Feb 23 21:16:24 localhost journal: internal error: dev->name buffer
overflow: 1:00:00.0

If you would prefer, I can resend the patch with the above error
pointing out that the patch fixes this issue alone.


[1]: https://patchwork.ozlabs.org/patch/798846/


> 
> Assuming that the VMD domain needs to be referenced in config somewhere
> in order to be useful, along with changing the type for pciDomain in
> docs/schemes/basictypes.rng, we would also need at least one new test
> case for the qemuxml2argv and qemuxml2xml tests (see the examples in the
> "hostdev-vfio-multidomain" and "net-hostdev-multidomain").
> 
I'd be happy to look into this when, if ever, we do actually support
passthrough of VMD child devices within the kernel.

> Also, do all versions of qemu support domains > 0x? If not, is there
> a feature that can be used to detect that support so we can have a
> capability bit for it and warn if someone tries to use such a domain
> when the installed version of qemu doesn't support it? (If there is no
> way to tell in advance, then we'll just have to live with reporting any
> qemu error after the fact)
> 
> 
At a first glance, QEMU somewhat supports it. Several domain references
use int, but others use uint16_t. I can take a look at cleaning this up,
but as stated above, VMD child devices won't actually be passed-through
in the foreseeable future. In the past I have done QEMU passthrough
tests with just the VMD endpoint (which resides on a 16-bit domain). The
guest is then responsible for child devices. This mode of operation is
the only one we officially support.

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

[libvirt] [PATCH python] rpm: assume python3 is always available

2017-08-09 Thread Daniel P. Berrange 
Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 17 -
 1 file changed, 17 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index fc30564..ed9f2bd 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -1,9 +1,4 @@
 
-%define with_python3 0
-%if 0%{?fedora} > 18
-%define with_python3 1
-%endif
-
 Summary: The libvirt virtualization API python2 binding
 Name: libvirt-python
 Version: @PY_VERSION@
@@ -16,11 +11,9 @@ BuildRequires: libvirt-devel >= @C_VERSION@
 BuildRequires: python-devel
 BuildRequires: python-nose
 BuildRequires: python-lxml
-%if %{with_python3}
 BuildRequires: python3-devel
 BuildRequires: python3-nose
 BuildRequires: python3-lxml
-%endif
 
 # Don't want provides for python shared objects
 %{?filter_provides_in: %filter_provides_in %{python_sitearch}/.*\.so}
@@ -32,7 +25,6 @@ written in the Python programming language to use the 
interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
 
-%if %{with_python3}
 %package -n libvirt-python3
 Summary: The libvirt virtualization API python3 binding
 Url: http://libvirt.org
@@ -44,7 +36,6 @@ The libvirt-python package contains a module that permits 
applications
 written in the Python programming language to use the interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
-%endif
 
 %prep
 %setup -q
@@ -56,21 +47,15 @@ find examples -type f -exec chmod 0644 \{\} \;
 
 %build
 CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build
-%if %{with_python3}
 CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
-%endif
 
 %install
 %{__python} setup.py install --skip-build --root=%{buildroot}
-%if %{with_python3}
 %{__python3} setup.py install --skip-build --root=%{buildroot}
-%endif
 
 %check
 %{__python} setup.py test
-%if %{with_python3}
 %{__python3} setup.py test
-%endif
 
 %files
 %defattr(-,root,root)
@@ -81,7 +66,6 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python2*/site-packages/libvirtmod*
 %{_libdir}/python2*/site-packages/*egg-info
 
-%if %{with_python3}
 %files -n libvirt-python3
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
@@ -95,6 +79,5 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python3*/site-packages/__pycache__/libvirtaio.cpython-*.py*
 %{_libdir}/python3*/site-packages/libvirtmod*
 %{_libdir}/python3*/site-packages/*egg-info
-%endif
 
 %changelog
-- 
2.13.3

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

[libvirt] [PATCH python] rpm: rename packages to python2-libvirt / python3-libvirt

2017-08-09 Thread Daniel P. Berrange 
This compiles with Fedora naming policy for python packages

Signed-off-by: Daniel P. Berrange 
---
 libvirt-python.spec.in | 28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index ed9f2bd..e4fb17a 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -25,14 +25,32 @@ written in the Python programming language to use the 
interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
 
-%package -n libvirt-python3
+%package -n python2-libvirt
+Summary: The libvirt virtualization API python2 binding
+Url: http://libvirt.org
+License: LGPLv2+
+Group: Development/Libraries
+%{?python_provide:%python_provide python2-libvirt}
+Provides: libvirt-python = %{version}-%{release}
+Obsoletes: libvirt-python < %{version}-%{release}
+
+%description -n python2-libvirt
+The libvirt-python2 package contains a module that permits applications
+written in the Python programming language to use the interface
+supplied by the libvirt library to use the virtualization capabilities
+of recent versions of Linux (and other OSes).
+
+%package -n python3-libvirt
 Summary: The libvirt virtualization API python3 binding
 Url: http://libvirt.org
 License: LGPLv2+
 Group: Development/Libraries
+%{?python_provide:%python_provide python3-libvirt}
+Provides: libvirt-python3 = %{version}-%{release}
+Obsoletes: libvirt-python3 < %{version}-%{release}
 
-%description -n libvirt-python3
-The libvirt-python package contains a module that permits applications
+%description -n python3-libvirt
+The libvirt-python3 package contains a module that permits applications
 written in the Python programming language to use the interface
 supplied by the libvirt library to use the virtualization capabilities
 of recent versions of Linux (and other OSes).
@@ -57,7 +75,7 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{__python} setup.py test
 %{__python3} setup.py test
 
-%files
+%files -n python2-libvirt
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
 %{_libdir}/python2*/site-packages/libvirt.py*
@@ -66,7 +84,7 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %{_libdir}/python2*/site-packages/libvirtmod*
 %{_libdir}/python2*/site-packages/*egg-info
 
-%files -n libvirt-python3
+%files -n python3-libvirt
 %defattr(-,root,root)
 %doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/
 %{_libdir}/python3*/site-packages/libvirt.py*
-- 
2.13.3

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

[libvirt] [PATCH python] Avoid comparing boolean and integers

2017-08-09 Thread Daniel P. Berrange 
Signed-off-by: Daniel P. Berrange 
---
 libvirt-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-utils.c b/libvirt-utils.c
index 727397d..0af13dc 100644
--- a/libvirt-utils.c
+++ b/libvirt-utils.c
@@ -108,7 +108,7 @@ virReallocN(void *ptrptr,
 return -1;
 }
 tmp = realloc(*(void**)ptrptr, size * count);
-if (!tmp && (size * count)) {
+if (!tmp && ((size * count) != 0)) {
 return -1;
 }
 *(void**)ptrptr = tmp;
-- 
2.13.3

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

Re: [libvirt] [PATCH v2] Rewrite the way mockable functions are handled.

2017-08-09 Thread Daniel P. Berrange 
On Fri, Aug 04, 2017 at 06:13:42PM +0100, Daniel P. Berrange wrote:
> Currently any mockable functions are marked with attributes
> noinline, noclone and weak. This prevents the compiler from
> optimizing away the impl of these functions.
> 
> It has an unfortunate side effect with the libtool convenience
> libraries, if executables directly link to them. For example
> virlockd, virlogd both link to libvirt_util.la  When this is
> done, the linker does not consider weak symbols as being
> undefined, so it never copies them into the final executable.
> 
> In this new approach, we stop annotating the headers entirely.
> Instead we create a weak function alias in the source.
> 
>int fooImpl(void) {
>   ..the real code..
>}
> 
>int foo(void) __attribute__((noinline, noclone, weak, alias("fooImpl"))
> 
> If any functions in the same file call "foo", this prevents the
> optimizer from inlining any part of fooImpl. When linking to the
> libtool convenience static library, we also get all the symbols
> present. Finally the test suite can just directly define a
> 'foo' function in its source, removing the need to use LD_PRELOAD
> (though removal of LD_PRELOADS is left for a future patch).
> 
> Signed-off-by: Daniel P. Berrange 

Self-NACK.

This breaks on OS-X because the linker doesn't support 'alias' or
'weak'.  For that matter it doesn't support LD_PRELOAD either, so
we need to avoid wrapping the functions on this platform.


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] apparmor, libvirt-qemu: Allow QEMU to gather information about available host resources.

2017-08-09 Thread Christian Ehrhardt 
We had the same rule for some time, it just is ordered later in our
submission stack and not yet pushed by me or Stefan for review.
But since we have the same rules for quite some time working fine I'm
clearly acking that.
Thanks intrigeri!

Acked-by: Christian Ehrhardt 

On Tue, Aug 8, 2017 at 11:57 PM, intrigeri 
wrote:

> ---
>  examples/apparmor/libvirt-qemu | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-
> qemu
> index f462d7428c..dcfb1a5985 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -169,3 +169,9 @@
>@{PROC}/device-tree/ r,
>@{PROC}/device-tree/** r,
>/sys/firmware/devicetree/** r,
> +
> +  # for gathering information about available host resources
> +  /sys/devices/system/cpu/ r,
> +  /sys/devices/system/node/ r,
> +  /sys/devices/system/node/node[0-9]*/meminfo r,
> +  /sys/module/vhost/parameters/max_mem_regions r,
> --
> 2.14.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY

2017-08-09 Thread Martin Kletzander 

On Mon, Aug 07, 2017 at 02:20:05PM +0200, Michal Privoznik wrote:

This API is definitely modifying state of @vm. Therefore it
should grab a job.

Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_driver.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f65f440..574c351ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7325,10 +7325,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;

+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
if (!vm->persistent) {
virReportError(VIR_ERR_OPERATION_INVALID,
   "%s", _("cannot undefine transient domain"));
-goto cleanup;
+goto endjob;
}

if (!virDomainObjIsActive(vm) &&
@@ -7338,15 +7341,15 @@ qemuDomainUndefineFlags(virDomainPtr dom,
   _("cannot delete inactive domain with %d "
 "snapshots"),
   nsnapshots);
-goto cleanup;
+goto endjob;
}
if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
-goto cleanup;
+goto endjob;
}

name = qemuDomainManagedSavePath(driver, vm);
if (name == NULL)
-goto cleanup;
+goto endjob;

if (virFileExists(name)) {
if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) {
@@ -7354,13 +7357,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
   _("Failed to remove domain managed "
 "save image"));
-goto cleanup;
+goto endjob;
}
} else {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
   _("Refusing to undefine while domain managed "
 "save image exists"));
-goto cleanup;
+goto endjob;
}
}

@@ -7372,17 +7375,17 @@ qemuDomainUndefineFlags(virDomainPtr dom,
virReportSystemError(errno,
 _("failed to remove nvram: %s"),
 vm->def->os.loader->nvram);
-goto cleanup;
+goto endjob;
}
} else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
   _("cannot delete inactive domain with nvram"));
-goto cleanup;
+goto endjob;
}
}

if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
-goto cleanup;
+goto endjob;

event = virDomainEventLifecycleNewFromObj(vm,
 VIR_DOMAIN_EVENT_UNDEFINED,
@@ -7399,6 +7402,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
qemuDomainRemoveInactive(driver, vm);


You cannot RemoveInactive with a job.  Not that it wouldn't make sense,
but the code is written that way (at least for now).  It takes another
MODIFY job itself.



ret = 0;
+ endjob:
+qemuDomainObjEndJob(driver, vm);

 cleanup:
VIR_FREE(name);
--
2.13.0

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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

2017-08-09 Thread Daniel P. Berrange 
On Mon, Aug 07, 2017 at 02:20:06PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
> 
> Currently, there's a bug when undefining a domain with NVRAM
> store. Basically, the unlink() of the NVRAM store file happens
> during the undefine procedure iff domain is inactive. So, if
> domain is running and undefine is called the file is left behind.
> It won't be removed in the domain cleanup process either
> (qemuProcessStop). One of the solutions is to remove if
> regardless of the domain state and rely on qemu having the file
> opened. This still has a downside that if the domain is defined
> back the NVRAM store file is going to be new, any changes to the
> current one are lost (just like with any other file that is
> deleted while a process has it opened). But is it really a
> downside?

We only unlink if the user explicitly gives VIR_DOMAIN_UNDEFINE_NVRAM,
so I think that "prolem" scenario you describe is exactly what the user
has asked for. ie not a bug - just don't pass VIR_DOMAIN_UNDEFINE_NVRAM
if they want to keep it around across an undefine+define pair.

> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_driver.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 574c351ae..992ae2a2e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7367,8 +7367,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>  }
>  }
>  
> -if (!virDomainObjIsActive(vm) &&
> -vm->def->os.loader && vm->def->os.loader->nvram &&
> +if (vm->def->os.loader &&
> +vm->def->os.loader->nvram &&
>  virFileExists(vm->def->os.loader->nvram)) {
>  if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
>  if (unlink(vm->def->os.loader->nvram) < 0) {

ACK

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 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

2017-08-09 Thread Michal Privoznik 
On 08/09/2017 03:02 PM, Daniel P. Berrange wrote:
> On Wed, Aug 09, 2017 at 02:55:36PM +0200, Michal Privoznik wrote:
>> On 08/09/2017 02:14 PM, Daniel P. Berrange wrote:
>>> On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1467245

 Currently, there's a bug when undefining a domain with NVRAM
 store. Basically, the unlink() of the NVRAM store file happens
 during the undefine procedure iff domain is inactive. So, if
 domain is running and undefine is called the file is left behind.
 It won't be removed in the domain cleanup process either
 (qemuProcessStop). To avoid this forbid undefining domain with
 NVRAM file.
>>>
>>> Why do we need to forbid it ? Even if QEMU still has an open
>>> file handle, it can continue to write to it after we unlink
>>> it.
>>>
>>>
>>
>> That's what my v1 does. Anyway, there's third option: just recently
>> Jirka added possibility to do some actions when domain is destroyed. He
>> needed it for some migration work, but the design is broad enough to fit
>> this problem too. What we can do is:
>>
>> if (flags & VIR_DOMAIN_UNDEFINE_NVRAM):
>>   if domain is running:
>> register the callback /* that merely just unlinks the file */
>>   else:
>> unlink
>> else:
>>   if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)):
>> error
>>
>>
>> What do you guys think of this one?
> 
> The callback will be lost if libvirtd restarts.

Ah, good point. In that case all I need is an ACK to v1 then ;-)

Michal

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

Re: [libvirt] [PATCH v2 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

2017-08-09 Thread Daniel P. Berrange 
On Wed, Aug 09, 2017 at 02:55:36PM +0200, Michal Privoznik wrote:
> On 08/09/2017 02:14 PM, Daniel P. Berrange wrote:
> > On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
> >>
> >> Currently, there's a bug when undefining a domain with NVRAM
> >> store. Basically, the unlink() of the NVRAM store file happens
> >> during the undefine procedure iff domain is inactive. So, if
> >> domain is running and undefine is called the file is left behind.
> >> It won't be removed in the domain cleanup process either
> >> (qemuProcessStop). To avoid this forbid undefining domain with
> >> NVRAM file.
> > 
> > Why do we need to forbid it ? Even if QEMU still has an open
> > file handle, it can continue to write to it after we unlink
> > it.
> > 
> > 
> 
> That's what my v1 does. Anyway, there's third option: just recently
> Jirka added possibility to do some actions when domain is destroyed. He
> needed it for some migration work, but the design is broad enough to fit
> this problem too. What we can do is:
> 
> if (flags & VIR_DOMAIN_UNDEFINE_NVRAM):
>   if domain is running:
> register the callback /* that merely just unlinks the file */
>   else:
> unlink
> else:
>   if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)):
> error
> 
> 
> What do you guys think of this one?

The callback will be lost if libvirtd 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/libvir-list

[libvirt] [PATCH] apparmor, libvirt-qemu: Allow QEMU to gather information about available host resources.

2017-08-09 Thread intrigeri 
---
 examples/apparmor/libvirt-qemu | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index f462d7428c..dcfb1a5985 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -169,3 +169,9 @@
   @{PROC}/device-tree/ r,
   @{PROC}/device-tree/** r,
   /sys/firmware/devicetree/** r,
+
+  # for gathering information about available host resources
+  /sys/devices/system/cpu/ r,
+  /sys/devices/system/node/ r,
+  /sys/devices/system/node/node[0-9]*/meminfo r,
+  /sys/module/vhost/parameters/max_mem_regions r,
-- 
2.14.0

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

Re: [libvirt] [PATCH v2 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

2017-08-09 Thread Michal Privoznik 
On 08/09/2017 02:14 PM, Daniel P. Berrange wrote:
> On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
>>
>> Currently, there's a bug when undefining a domain with NVRAM
>> store. Basically, the unlink() of the NVRAM store file happens
>> during the undefine procedure iff domain is inactive. So, if
>> domain is running and undefine is called the file is left behind.
>> It won't be removed in the domain cleanup process either
>> (qemuProcessStop). To avoid this forbid undefining domain with
>> NVRAM file.
> 
> Why do we need to forbid it ? Even if QEMU still has an open
> file handle, it can continue to write to it after we unlink
> it.
> 
> 

That's what my v1 does. Anyway, there's third option: just recently
Jirka added possibility to do some actions when domain is destroyed. He
needed it for some migration work, but the design is broad enough to fit
this problem too. What we can do is:

if (flags & VIR_DOMAIN_UNDEFINE_NVRAM):
  if domain is running:
register the callback /* that merely just unlinks the file */
  else:
unlink
else:
  if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)):
error


What do you guys think of this one?

Michal

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

Re: [libvirt] [PATCH v2 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

2017-08-09 Thread Martin Kletzander 

On Wed, Aug 09, 2017 at 01:14:59PM +0100, Daniel P. Berrange wrote:

On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1467245

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). To avoid this forbid undefining domain with
NVRAM file.


Why do we need to forbid it ? Even if QEMU still has an open
file handle, it can continue to write to it after we unlink
it.



It was a suggestion, because that's what will people expect, IMHO.

Feel free to go with the first version if that's what Dan prefers.



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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

2017-08-09 Thread Daniel P. Berrange 
On Wed, Aug 09, 2017 at 02:00:06PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
> 
> Currently, there's a bug when undefining a domain with NVRAM
> store. Basically, the unlink() of the NVRAM store file happens
> during the undefine procedure iff domain is inactive. So, if
> domain is running and undefine is called the file is left behind.
> It won't be removed in the domain cleanup process either
> (qemuProcessStop). To avoid this forbid undefining domain with
> NVRAM file.

Why do we need to forbid it ? Even if QEMU still has an open
file handle, it can continue to write to it after we unlink
it.


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 2/2] qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

2017-08-09 Thread Michal Privoznik 
https://bugzilla.redhat.com/show_bug.cgi?id=1467245

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). To avoid this forbid undefining domain with
NVRAM file.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 574c351ae..dc5b924ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7367,10 +7367,15 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 }
 }
 
-if (!virDomainObjIsActive(vm) &&
-vm->def->os.loader && vm->def->os.loader->nvram &&
+if (vm->def->os.loader &&
+vm->def->os.loader->nvram &&
 virFileExists(vm->def->os.loader->nvram)) {
 if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
+if (virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("cannot delete active domain with nvram"));
+goto endjob;
+}
 if (unlink(vm->def->os.loader->nvram) < 0) {
 virReportSystemError(errno,
  _("failed to remove nvram: %s"),
-- 
2.13.0

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

[libvirt] [PATCH v2 1/2] qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY

2017-08-09 Thread Michal Privoznik 
This API is definitely modifying state of @vm. Therefore it
should grab a job.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f65f440..574c351ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7325,10 +7325,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
 if (!vm->persistent) {
 virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cannot undefine transient domain"));
-goto cleanup;
+goto endjob;
 }
 
 if (!virDomainObjIsActive(vm) &&
@@ -7338,15 +7341,15 @@ qemuDomainUndefineFlags(virDomainPtr dom,
_("cannot delete inactive domain with %d "
  "snapshots"),
nsnapshots);
-goto cleanup;
+goto endjob;
 }
 if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
-goto cleanup;
+goto endjob;
 }
 
 name = qemuDomainManagedSavePath(driver, vm);
 if (name == NULL)
-goto cleanup;
+goto endjob;
 
 if (virFileExists(name)) {
 if (flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE) {
@@ -7354,13 +7357,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to remove domain managed "
  "save image"));
-goto cleanup;
+goto endjob;
 }
 } else {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Refusing to undefine while domain managed "
  "save image exists"));
-goto cleanup;
+goto endjob;
 }
 }
 
@@ -7372,17 +7375,17 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 virReportSystemError(errno,
  _("failed to remove nvram: %s"),
  vm->def->os.loader->nvram);
-goto cleanup;
+goto endjob;
 }
 } else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("cannot delete inactive domain with nvram"));
-goto cleanup;
+goto endjob;
 }
 }
 
 if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
-goto cleanup;
+goto endjob;
 
 event = virDomainEventLifecycleNewFromObj(vm,
  VIR_DOMAIN_EVENT_UNDEFINED,
@@ -7399,6 +7402,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
 qemuDomainRemoveInactive(driver, vm);
 
 ret = 0;
+ endjob:
+qemuDomainObjEndJob(driver, vm);
 
  cleanup:
 VIR_FREE(name);
-- 
2.13.0

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

[libvirt] [PATCH v2 0/2] qemuDomainUndefineFlags: Two fixes

2017-08-09 Thread Michal Privoznik 
v2 of:

https://www.redhat.com/archives/libvir-list/2017-August/msg00221.html

diff to v1:
- instead of removing the file, abort the whole operation

Michal Privoznik (2):
  qemuDomainUndefineFlags: Grab QEMU_JOB_MODIFY
  qemuDomainUndefineFlags: Forbid undefine of active domain with NVRAM

 src/qemu/qemu_driver.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

-- 
2.13.0

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

Re: [libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

2017-08-09 Thread Martin Kletzander 

On Wed, Aug 09, 2017 at 01:13:02PM +0200, Michal Privoznik wrote:

On 08/09/2017 11:41 AM, Martin Kletzander wrote:

On Mon, Aug 07, 2017 at 02:20:06PM +0200, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1467245

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). One of the solutions is to remove if
regardless of the domain state and rely on qemu having the file
opened. This still has a downside that if the domain is defined
back the NVRAM store file is going to be new, any changes to the
current one are lost (just like with any other file that is
deleted while a process has it opened). But is it really a
downside?



It might be.  Why don't we disable removing it when the domain is
running?  We have some precedence for this.  The place which already
deals with this possibility is tools/virsh-domain.c in the cmdUndefine()
where we handle --remove-all-storage.  If you look at the help of that
command it also says:

 --nvram  remove nvram file, if inactive

And that makes sense to me.  What doesn't, on the other hand, is that it
also says:

 --keep-nvram keep nvram file, if inactive

I don't get the "if inactive" there.  But I'm not going to check who
pushed that.  At least not again =)


Okay, I'll write a patch that:

a) forbids undefine for active domains, unless
b) KEEP_NVRAM flag is specified.



c) or there is no nvram =)

Yeah, well, that's caused by the unfortunate design of the way nvram is
handled by qemu and qemu driver.

Thanks


Michal

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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

2017-08-09 Thread Michal Privoznik 
On 08/09/2017 11:41 AM, Martin Kletzander wrote:
> On Mon, Aug 07, 2017 at 02:20:06PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1467245
>>
>> Currently, there's a bug when undefining a domain with NVRAM
>> store. Basically, the unlink() of the NVRAM store file happens
>> during the undefine procedure iff domain is inactive. So, if
>> domain is running and undefine is called the file is left behind.
>> It won't be removed in the domain cleanup process either
>> (qemuProcessStop). One of the solutions is to remove if
>> regardless of the domain state and rely on qemu having the file
>> opened. This still has a downside that if the domain is defined
>> back the NVRAM store file is going to be new, any changes to the
>> current one are lost (just like with any other file that is
>> deleted while a process has it opened). But is it really a
>> downside?
>>
> 
> It might be.  Why don't we disable removing it when the domain is
> running?  We have some precedence for this.  The place which already
> deals with this possibility is tools/virsh-domain.c in the cmdUndefine()
> where we handle --remove-all-storage.  If you look at the help of that
> command it also says:
> 
>  --nvram  remove nvram file, if inactive
> 
> And that makes sense to me.  What doesn't, on the other hand, is that it
> also says:
> 
>  --keep-nvram keep nvram file, if inactive
> 
> I don't get the "if inactive" there.  But I'm not going to check who
> pushed that.  At least not again =)

Okay, I'll write a patch that:

a) forbids undefine for active domains, unless
b) KEEP_NVRAM flag is specified.

Michal

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

Re: [libvirt] [PATCH 2/2] qemuDomainUndefineFlags: unlink nvram file regardless of domain state

2017-08-09 Thread Martin Kletzander 

On Mon, Aug 07, 2017 at 02:20:06PM +0200, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1467245

Currently, there's a bug when undefining a domain with NVRAM
store. Basically, the unlink() of the NVRAM store file happens
during the undefine procedure iff domain is inactive. So, if
domain is running and undefine is called the file is left behind.
It won't be removed in the domain cleanup process either
(qemuProcessStop). One of the solutions is to remove if
regardless of the domain state and rely on qemu having the file
opened. This still has a downside that if the domain is defined
back the NVRAM store file is going to be new, any changes to the
current one are lost (just like with any other file that is
deleted while a process has it opened). But is it really a
downside?



It might be.  Why don't we disable removing it when the domain is
running?  We have some precedence for this.  The place which already
deals with this possibility is tools/virsh-domain.c in the cmdUndefine()
where we handle --remove-all-storage.  If you look at the help of that
command it also says:

 --nvram  remove nvram file, if inactive

And that makes sense to me.  What doesn't, on the other hand, is that it
also says:

 --keep-nvram keep nvram file, if inactive

I don't get the "if inactive" there.  But I'm not going to check who
pushed that.  At least not again =)


Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 574c351ae..992ae2a2e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7367,8 +7367,8 @@ qemuDomainUndefineFlags(virDomainPtr dom,
}
}

-if (!virDomainObjIsActive(vm) &&
-vm->def->os.loader && vm->def->os.loader->nvram &&
+if (vm->def->os.loader &&
+vm->def->os.loader->nvram &&
virFileExists(vm->def->os.loader->nvram)) {
if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
if (unlink(vm->def->os.loader->nvram) < 0) {
--
2.13.0

--
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 v2] Update to latest keycodemapdb content

2017-08-09 Thread Daniel P. Berrange 
Signed-off-by: Daniel P. Berrange 
---
 src/Makefile.am| 2 +-
 src/keycodemapdb   | 2 +-
 src/util/virkeycode.c  | 5 ++---
 tests/virkeycodetest.c | 4 ++--
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index b8e875482..45b58c0ad 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -252,7 +252,7 @@ util/virkey%.7: util/virkey%.pod
rm -f $@-t1 && \
mv $@-t2 $@
 
-KEYCODES = linux osx atset1 atset2 atset3 xt xtkbd usb win32 rfb
+KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 rfb
 KEYNAMES = linux osx win32
 
 KEYTABLES = \
diff --git a/src/keycodemapdb b/src/keycodemapdb
index 7bf5710b2..267157b96 16
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 7bf5710b22aa8d58b7eeaaf3dc6960c26cade4f0
+Subproject commit 267157b96c62b5445de9cddd21de42fcd943ffe6
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index e09aaadaa..eda263218 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -30,7 +30,6 @@
 #include "virkeycodetable_rfb.h"
 #include "virkeycodetable_usb.h"
 #include "virkeycodetable_win32.h"
-#include "virkeycodetable_xt.h"
 #include "virkeycodetable_xtkbd.h"
 #include "virkeynametable_linux.h"
 #include "virkeynametable_osx.h"
@@ -44,7 +43,8 @@ static const char **virKeymapNames[VIR_KEYCODE_SET_LAST] = {
 
 static const unsigned short *virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 [VIR_KEYCODE_SET_LINUX] = virKeyCodeTable_linux,
-[VIR_KEYCODE_SET_XT] = virKeyCodeTable_xt,
+/* XT is same as AT Set1 - it was included by mistake */
+[VIR_KEYCODE_SET_XT] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET1] = virKeyCodeTable_atset1,
 [VIR_KEYCODE_SET_ATSET2] = virKeyCodeTable_atset2,
 [VIR_KEYCODE_SET_ATSET3] = virKeyCodeTable_atset3,
@@ -57,7 +57,6 @@ static const unsigned short 
*virKeymapValues[VIR_KEYCODE_SET_LAST] = {
 
 #define VIR_KEYMAP_ENTRY_MAX ARRAY_CARDINALITY(virKeyCodeTable_linux)
 
-verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_xt));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset1));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset2));
 verify(VIR_KEYMAP_ENTRY_MAX == ARRAY_CARDINALITY(virKeyCodeTable_atset3));
diff --git a/tests/virkeycodetest.c b/tests/virkeycodetest.c
index d754385be..4092aa119 100644
--- a/tests/virkeycodetest.c
+++ b/tests/virkeycodetest.c
@@ -56,9 +56,9 @@ static int testKeycodeMapping(const void *data 
ATTRIBUTE_UNUSED)
 TRANSLATE(LINUX, USB, 111, 76);
 TRANSLATE(LINUX, RFB, 88, 88);
 TRANSLATE(LINUX, RFB, 160, 163);
-TRANSLATE(ATSET2, ATSET3, 259, 55);
+TRANSLATE(ATSET2, ATSET3, 131, 55);
 TRANSLATE(OSX, WIN32, 90, 131);
-TRANSLATE(OSX, ATSET1, 90, 0);
+TRANSLATE(OSX, ATSET1, 90, 90);
 TRANSLATE(OSX, ATSET1, 3200, -1);
 
 #undef TRANSLATE
-- 
2.13.3

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

Re: [libvirt] [PATCH] Update to latest keycodemapdb content

2017-08-09 Thread Daniel P. Berrange 
On Tue, Aug 08, 2017 at 02:09:17PM +0200, Andrea Bolognani wrote:
> On Mon, 2017-08-07 at 14:38 +0100, Daniel P. Berrange wrote:
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  src/Makefile.am   | 2 +-
> >  src/keycodemapdb  | 2 +-
> >  src/util/virkeycode.c | 5 ++---
> >  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> The changes look sane, but the test suite is not happy:
> 
>   $ VIR_TEST_DEBUG=1 ./tests/virkeycodetest
>   TEST: virkeycodetest
>1) Keycode mapping ... Translating 259 from ATSET2
>   to ATSET3, got -1 want 55
>   FAILED
> 
> Seems unrelated to your changes though, perhaps a
> regression in keycodemapdb?

No, its a fix in keycodemapdb. A sheer good fortune, the scancodes we
decided to test were incorrectly defined. So when we fixed the mappings
in keycodemapdb, we broke the tests :-)

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