[libvirt] [jenkins-ci PATCH] guests: Use OpenJDK 8 on Debian 8 too
Signed-off-by: Andrea Bolognani --- guests/files/jessie-backports.preferences | 3 +++ guests/files/jessie-backports.sources | 1 + guests/tasks/base.yml | 20 guests/vars/mappings.yml | 1 - 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 guests/files/jessie-backports.preferences create mode 100644 guests/files/jessie-backports.sources diff --git a/guests/files/jessie-backports.preferences b/guests/files/jessie-backports.preferences new file mode 100644 index 000..5e657f6 --- /dev/null +++ b/guests/files/jessie-backports.preferences @@ -0,0 +1,3 @@ +Package: openjdk-8-jre-headless java-common ca-certificates-java +Pin: release a=jessie-backports +Pin-Priority: 900 diff --git a/guests/files/jessie-backports.sources b/guests/files/jessie-backports.sources new file mode 100644 index 000..6e6d261 --- /dev/null +++ b/guests/files/jessie-backports.sources @@ -0,0 +1 @@ +deb http://deb.debian.org/debian/ jessie-backports main diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml index 6acd967..a25420a 100644 --- a/guests/tasks/base.yml +++ b/guests/tasks/base.yml @@ -33,6 +33,26 @@ - ( os_name == 'CentOS' or os_name == 'Fedora' ) +- name: Enable jessie-backports repository + copy: +src: files/jessie-backports.sources +dest: /etc/apt/sources.list.d/jessie-backports.list +owner: root +group: root + when: +- os_name == 'Debian' +- os_version == '8' + +- name: Configure APT pinning for jessie-backports + copy: +src: files/jessie-backports.preferences +dest: /etc/apt/preferences.d/jessie-backports +owner: root +group: root + when: +- os_name == 'Debian' +- os_version == '8' + - name: Bootstrap the package module command: apt-get install -y python-apt args: diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index cae9d23..d620b5d 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -175,7 +175,6 @@ mappings: deb: openjdk-8-jre-headless pkg: openjdk8-jre rpm: java-1.8.0-openjdk-headless -Debian8: openjdk-7-jre-headless Ubuntu12: openjdk-7-jre-headless Ubuntu14: openjdk-7-jre-headless -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] guests: Reduce boot delay for FreeBSD
Set it to 1 second instead of the default 10 seconds. This brings
it in line with Linux guests and makes boot faster.
Signed-off-by: Andrea Bolognani
---
guests/tasks/base.yml | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml
index a25420a..a71e66d 100644
--- a/guests/tasks/base.yml
+++ b/guests/tasks/base.yml
@@ -181,9 +181,12 @@
- name: Configure the FreeBSD bootloader
lineinfile:
path: /boot/loader.conf
-regexp: '^console=.*$'
-line: 'console="comconsole"'
+regexp: '^{{ item.key }}=.*$'
+line: '{{ item.key }}="{{ item.value }}"'
create: yes
backup: yes
+ with_items:
+- { key: 'console', value: 'comconsole' }
+- { key: 'autoboot_delay', value: '1' }
when:
- os_name == 'FreeBSD'
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/6] enable numa configuration before machine_init() from HMP/QMP
On Fri, Oct 20, 2017 at 10:07:27AM +0100, Daniel P. Berrange wrote: > On Thu, Oct 19, 2017 at 05:56:49PM -0200, Eduardo Habkost wrote: > > On Thu, Oct 19, 2017 at 04:28:59PM +0100, Daniel P. Berrange wrote: > > > On Thu, Oct 19, 2017 at 11:21:22AM -0400, Igor Mammedov wrote: > > > > - Original Message - > > > > > From: "Daniel P. Berrange" > > > > > To: "Igor Mammedov" > > > > > Cc: "peter maydell" , pkre...@redhat.com, > > > > > ehabk...@redhat.com, coh...@redhat.com, > > > > > qemu-de...@nongnu.org, arm...@redhat.com, pbonz...@redhat.com, > > > > > da...@gibson.dropbear.id.au > > > > > Sent: Wednesday, October 18, 2017 5:30:10 PM > > > > > Subject: Re: [Qemu-devel] [RFC 0/6] enable numa configuration before > > > > > machine_init() from HMP/QMP > > > > > > > > > > On Tue, Oct 17, 2017 at 06:06:35PM +0200, Igor Mammedov wrote: > > > > > > On Tue, 17 Oct 2017 16:07:59 +0100 > > > > > > "Daniel P. Berrange" wrote: > > > > > > > > > > > > > On Tue, Oct 17, 2017 at 09:27:02AM +0200, Igor Mammedov wrote: > > > > > > > > On Mon, 16 Oct 2017 17:36:36 +0100 > > > > > > > > "Daniel P. Berrange" wrote: > > > > > > > > > > > > > > > > > On Mon, Oct 16, 2017 at 06:22:50PM +0200, Igor Mammedov wrote: > > > > > > > > > > Series allows to configure NUMA mapping at runtime using > > > > > > > > > > QMP/HMP > > > > > > > > > > interface. For that to happen it introduces a new '-paused' > > > > > > > > > > CLI > > > > > > > > > > option > > > > > > > > > > which allows to pause QEMU before machine_init() is run and > > > > > > > > > > adds new set-numa-node HMP/QMP commands which in conjuction > > > > > > > > > > with > > > > > > > > > > info hotpluggable-cpus/query-hotpluggable-cpus allow to > > > > > > > > > > configure > > > > > > > > > > NUMA mapping for cpus. > > > > > > > > > > > > > > > > > > What's the problem we're seeking solve here compared to what > > > > > > > > > we > > > > > > > > > currently > > > > > > > > > do for NUMA configuration ? > > > > > > > > From RHBZ1382425 > > > > > > > > " > > > > > > > > Current -numa CLI interface is quite limited in terms that > > > > > > > > allow map > > > > > > > > CPUs to NUMA nodes as it requires to provide cpu_index values > > > > > > > > which > > > > > > > > are non obvious and depend on machine/arch. As result libvirt > > > > > > > > has to > > > > > > > > assume/re-implement cpu_index allocation logic to provide valid > > > > > > > > values for -numa cpus=... QEMU CLI option. > > > > > > > > > > > > > > In broad terms, this problem applies to every device / object > > > > > > > libvirt > > > > > > > asks QEMU to create. For everything else libvirt is able to > > > > > > > assign a > > > > > > > "id" string, which is can then use to identify the thing later. > > > > > > > The > > > > > > > CPU stuff is different because libvirt isn't able to provide 'id' > > > > > > > strings for each CPU - QEMU generates a psuedo-id internally which > > > > > > > libvirt has to infer. The latter is the same problem we had with > > > > > > > devices before '-device' was introduced allowing 'id' naming. > > > > > > > > > > > > > > IMHO we should take the same approach with CPUs and start > > > > > > > modelling > > > > > > > the individual CPUs as something we can explicitly create with > > > > > > > -object > > > > > > > or -device. That way libvirt can assign names and does not have to > > > > > > > care about CPU index values, and it all works just the same way as > > > > > > > any other devices / object we create > > > > > > > > > > > > > > ie instead of: > > > > > > > > > > > > > > -smp 8,sockets=4,cores=2,threads=1 > > > > > > > -numa node,nodeid=0,cpus=0-3 > > > > > > > -numa node,nodeid=1,cpus=4-7 > > > > > > > > > > > > > > we could do: > > > > > > > > > > > > > > -object numa-node,id=numa0 > > > > > > > -object numa-node,id=numa1 > > > > > > > -object cpu,id=cpu0,node=numa0,socket=0,core=0,thread=0 > > > > > > > -object cpu,id=cpu1,node=numa0,socket=0,core=1,thread=0 > > > > > > > -object cpu,id=cpu2,node=numa0,socket=1,core=0,thread=0 > > > > > > > -object cpu,id=cpu3,node=numa0,socket=1,core=1,thread=0 > > > > > > > -object cpu,id=cpu4,node=numa1,socket=2,core=0,thread=0 > > > > > > > -object cpu,id=cpu5,node=numa1,socket=2,core=1,thread=0 > > > > > > > -object cpu,id=cpu6,node=numa1,socket=3,core=0,thread=0 > > > > > > > -object cpu,id=cpu7,node=numa1,socket=3,core=1,thread=0 > > > > > > the follow up question would be where do "socket=3,core=1,thread=0" > > > > > > come from, currently these options are the function of > > > > > > (-M foo -smp ...) and can be queried vi query-hotpluggble-cpus at > > > > > > runtime after qemu parses -M and -smp options. > > > > > > > > > > NB, I realize my example was open to mis-interpretation. The values > > > > > I'm > > > > > illustrating here for socket=3,core=1,thread=0 and *not* ID values, > > > > > they > > > > > are a plain enumeration of values. ie this is s
Re: [libvirt] [jenkins-ci PATCH] guests: disable glusterfs on FreeBSD
On Fri, 2017-10-20 at 21:49 +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > guests/vars/mappings.yml | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml > index cae9d23..828690e 100644 > --- a/guests/vars/mappings.yml > +++ b/guests/vars/mappings.yml > @@ -118,7 +118,6 @@ mappings: > >glusterfs: > deb: glusterfs-client > -pkg: glusterfs > rpm: glusterfs-api-devel I'm convinced this should work, but since it's blocking the rest of the CI we can change it now and if appropriate revert it later. Reviewed-by: Andrea Bolognani -- 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] guests: use su instead of sudo in rc.local to start Jenkins agent
On Fri, 2017-10-20 at 21:36 +0200, Pavel Hrdina wrote: > On FreeBSD the sudo command cleans the new environment too much > and Jenkins is not able to run properly. > > Signed-off-by: Pavel Hrdina > --- > guests/group_vars/all/main.yml | 2 +- > guests/host_vars/libvirt-freebsd-10/main.yml | 2 +- > guests/host_vars/libvirt-freebsd-11/main.yml | 2 +- > guests/tasks/jenkins.yml | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) It still really puzzles me that sudo doesn't work properly in this context... In any case Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] guests: disable glusterfs on FreeBSD
Signed-off-by: Pavel Hrdina --- guests/vars/mappings.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index cae9d23..828690e 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -118,7 +118,6 @@ mappings: glusterfs: deb: glusterfs-client -pkg: glusterfs rpm: glusterfs-api-devel gnome-common: -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] guests: use su instead of sudo in rc.local to start Jenkins agent
On FreeBSD the sudo command cleans the new environment too much
and Jenkins is not able to run properly.
Signed-off-by: Pavel Hrdina
---
guests/group_vars/all/main.yml | 2 +-
guests/host_vars/libvirt-freebsd-10/main.yml | 2 +-
guests/host_vars/libvirt-freebsd-11/main.yml | 2 +-
guests/tasks/jenkins.yml | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/guests/group_vars/all/main.yml b/guests/group_vars/all/main.yml
index d24af59..9bf5d05 100644
--- a/guests/group_vars/all/main.yml
+++ b/guests/group_vars/all/main.yml
@@ -12,4 +12,4 @@ jenkins_url: https://ci.centos.org/computer/{{
inventory_hostname }}/slave-agent
bash: /bin/bash
java: /usr/bin/java
make: /usr/bin/make
-sudo: /usr/bin/sudo
+su: /bin/su
diff --git a/guests/host_vars/libvirt-freebsd-10/main.yml
b/guests/host_vars/libvirt-freebsd-10/main.yml
index 1547802..2931fe3 100644
--- a/guests/host_vars/libvirt-freebsd-10/main.yml
+++ b/guests/host_vars/libvirt-freebsd-10/main.yml
@@ -4,7 +4,7 @@ ansible_python_interpreter: /usr/local/bin/python2
bash: /usr/local/bin/bash
java: /usr/local/bin/java
make: /usr/local/bin/gmake
-sudo: /usr/local/bin/sudo
+su: /usr/bin/su
projects:
- base
diff --git a/guests/host_vars/libvirt-freebsd-11/main.yml
b/guests/host_vars/libvirt-freebsd-11/main.yml
index 1547802..2931fe3 100644
--- a/guests/host_vars/libvirt-freebsd-11/main.yml
+++ b/guests/host_vars/libvirt-freebsd-11/main.yml
@@ -4,7 +4,7 @@ ansible_python_interpreter: /usr/local/bin/python2
bash: /usr/local/bin/bash
java: /usr/local/bin/java
make: /usr/local/bin/gmake
-sudo: /usr/local/bin/sudo
+su: /usr/bin/su
projects:
- base
diff --git a/guests/tasks/jenkins.yml b/guests/tasks/jenkins.yml
index a1b8f46..87ebafa 100644
--- a/guests/tasks/jenkins.yml
+++ b/guests/tasks/jenkins.yml
@@ -19,7 +19,7 @@
create: yes
backup: yes
regexp: '^nohup.*jenkins.*java.*slave\.jar.*&$'
-line: "nohup {{ sudo }} -u jenkins {{ bash }} -l -c '{{ java }} -jar
/home/jenkins/slave.jar -jnlpUrl \"{{ jenkins_url }}\" -secret \"{{
jenkins_secret }}\"' >/var/log/jenkins.log 2>&1 &"
+line: "nohup {{ su }} - jenkins -c '{{ java }} -jar
/home/jenkins/slave.jar -jnlpUrl \"{{ jenkins_url }}\" -secret \"{{
jenkins_secret }}\"' >/var/log/jenkins.log 2>&1 &"
insertbefore: '^exit .*$'
when:
- ansible_service_mgr != 'systemd'
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Question about the host-model CPU mode
On 10/20/2017 04:12 PM, Christian Borntraeger wrote:
>
>
> On 10/20/2017 04:06 PM, David Hildenbrand wrote:
>> On 20.10.2017 16:02, Christian Borntraeger wrote:
>>>
>>>
>>> On 10/20/2017 03:51 PM, David Hildenbrand wrote:
>>> [...]
> The problem goes much further.
> A fresh guest with
>
>
> hvm
>
>
>
> does not start. No migration from an older system is necessary.
>
Yes, as stated in the documentation "copying host CPU definition from
capabilities XML" this can not work. And it works just as documented.
Not saying this is a nice thing :)
I think we should try to fix gs_allowed (if possible) and avoid
something like that in the future. This would avoid other complexity
involved when suddenly having X host models.
>>>
>>> Maybe this one is really a proper fix. It will allow the guest to start
>>> and on migration the cpu model will complain if the target cannot provide
>>> gs.
>>> Similar things can happen if - for example - the host kernel lacks some
>>> features.
>>
>> Right, just what I had in mind.
>>
>>>
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 77169bb..97a08fa 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc,
>>> void *data)
>>> s390mc->ri_allowed = true;
>>> s390mc->cpu_model_allowed = true;
>>> s390mc->css_migration_enabled = true;
>>> -s390mc->gs_allowed = true;
>>> mc->init = ccw_init;
>>> mc->reset = s390_machine_reset;
>>> mc->hot_add_cpu = s390_hot_add_cpu;
>>> @@ -509,12 +508,6 @@ bool cpu_model_allowed(void)
>>> return get_machine_class()->cpu_model_allowed;
>>> }
>>>
>>> -bool gs_allowed(void)
>>> -{
>>> -/* for "none" machine this results in true */
>>> -return get_machine_class()->gs_allowed;
>>> -}
>>> -
>>> static char *machine_get_loadparm(Object *obj, Error **errp)
>>> {
>>> S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>> @@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass
>>> *mc)
>>> {
>>> S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>>>
>>> -s390mc->gs_allowed = false;
>>> ccw_machine_2_10_class_options(mc);
>>> SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
>>> s390mc->css_migration_enabled = false;
>>> diff --git a/include/hw/s390x/s390-virtio-ccw.h
>>> b/include/hw/s390x/s390-virtio-ccw.h
>>> index a9a90c2..1de53b0 100644
>>> --- a/include/hw/s390x/s390-virtio-ccw.h
>>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>>> @@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass {
>>> bool ri_allowed;
>>> bool cpu_model_allowed;
>>> bool css_migration_enabled;
>>> -bool gs_allowed;
>>> } S390CcwMachineClass;
>>>
>>> /* runtime-instrumentation allowed by the machine */
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index a0d5052..3f13fc2 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>> cap_ri = 1;
>>> }
>>> }
>>> -if (gs_allowed()) {
>>> +if (cpu_model_allowed()) {
>>> if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
>>> cap_gs = 1;
>>> }
>>>
>>
>> And the last hunk makes sure we keep same handling for machines without
>> CPU model support and therefore no way to mask support. For all recent
>> machines, we expect CPU model checks to be in place.
>>
>> Will have to think about this a bit more. Will you send this as a proper
>> patch?
>
> After thinking about it :-)
>
I intend to put some brain-power in this too. Probably next week.
My general impression is, that I have a at places different understanding
of how things should work compared to David. Especially when it comes
to this concept of persistent copying, and also an end-user-digestible
definition of what host-model does. (I think this with copying capabilities
from whatever xml which is subject to convoluted caching is a bit too
hard to digest for an end user not involved in the development of qemu
and libvirt).
I had a conversation with Boris a couple of hours ago, and it seems, things
are pretty convoluted.
If I understand the train of thought here (David) it can be summarized like
this:
For a certain QEMU binary no aspect of the cpu-models may depend on the
machine type. In particular, compat properties and compat handling is
not alowed to alter cpu-models (whether the available cpu models nor the
capabilities of each of these).
This we would have to make a part of the external interface. That way
one could be sure that the 'cpu capabilities' are machine-type independent
(that is, the same for all the machine types).
Or did I get this completely wrong? (Based on the answer branches my
think).
Halil
--
libvir-list mailing list
libvir-list@redhat.com
https://www
[libvirt] [PATCH] conf: Rename [n]macs and maxmacs to [n]names and maxnames
To avoid further confusion - rename the array elements to what they are.
Signed-off-by: John Ferlan
---
Pushing as trivial as noted by:
https://www.redhat.com/archives/libvir-list/2017-October/msg00981.html
src/conf/virinterfaceobj.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 21d76e7507..f90c0bd9c4 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -160,9 +160,9 @@ virInterfaceObjListNew(void)
struct _virInterfaceObjFindMACData {
const char *matchStr;
bool error;
-int nmacs;
-int maxmacs;
-char **const macs;
+int nnames;
+int maxnames;
+char **const names;
};
static int
@@ -176,17 +176,17 @@ virInterfaceObjListFindByMACStringCb(void *payload,
if (data->error)
return 0;
-if (data->nmacs == data->maxmacs)
+if (data->nnames == data->maxnames)
return 0;
virObjectLock(obj);
if (STRCASEEQ(obj->def->mac, data->matchStr)) {
-if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) {
+if (VIR_STRDUP(data->names[data->nnames], obj->def->name) < 0) {
data->error = true;
goto cleanup;
}
-data->nmacs++;
+data->nnames++;
}
cleanup:
@@ -203,9 +203,9 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr
interfaces,
{
struct _virInterfaceObjFindMACData data = { .matchStr = mac,
.error = false,
-.nmacs = 0,
-.maxmacs = maxmatches,
-.macs = matches };
+.nnames = 0,
+.maxnames = maxmatches,
+.names = matches };
virObjectRWLockRead(interfaces);
virHashForEach(interfaces->objsName, virInterfaceObjListFindByMACStringCb,
@@ -215,11 +215,11 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr
interfaces,
if (data.error)
goto error;
-return data.nmacs;
+return data.nnames;
error:
-while (--data.nmacs >= 0)
-VIR_FREE(data.macs[data.nmacs]);
+while (--data.nnames >= 0)
+VIR_FREE(data.names[data.nnames]);
return -1;
}
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: duplicate interface name instead of MAC provided to lookup the interface
On 10/20/2017 10:28 AM, Pavel Hrdina wrote:
> On Fri, Oct 20, 2017 at 04:25:13PM +0200, Andrea Bolognani wrote:
>> On Fri, 2017-10-20 at 16:07 +0200, Pavel Hrdina wrote:
>>> Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129.
>>> Found by running libvirt-perl tests.
>>>
>>> Signed-off-by: Pavel Hrdina
>>> ---
>>> src/conf/virinterfaceobj.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
>>> index a6814a6aee..21d76e7507 100644
>>> --- a/src/conf/virinterfaceobj.c
>>> +++ b/src/conf/virinterfaceobj.c
>>> @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload,
>>> virObjectLock(obj);
>>>
>>> if (STRCASEEQ(obj->def->mac, data->matchStr)) {
>>> -if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) {
>>> +if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) {
>>> data->error = true;
>>> goto cleanup;
>>> }
>>
>> Reviewed-by: Andrea Bolognani
>
> Thanks
>
>> As an aside, I think 'macs' is a pretty bad name for the array, since
>> we're not storing MAC addresses but rather interface names, so using
>> either 'matches' or 'names' would be much better IMHO. Do you want me
>> to send a follow-up patch performing the rename?
>
> That would be good idea, feel free to send it and probably push it as
> trivial.
>
> Pavel
>
Doh - thanks for catching this... Who knew the perl tests would do
better than the libvirt tests.
As for [n]macs and naming - that's why I preferred the non-descript
nElems, elems, and maxelems .
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] projects: build on new debian-9 and freebsd-11 guests
On Fri, 2017-10-20 at 20:40 +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > projects/libvirt.yaml | 3 +++ > 1 file changed, 3 insertions(+) Good stuff :) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] projects: build on new debian-9 and freebsd-11 guests
Signed-off-by: Pavel Hrdina --- projects/libvirt.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/projects/libvirt.yaml b/projects/libvirt.yaml index a70a58c..1b31c1b 100644 --- a/projects/libvirt.yaml +++ b/projects/libvirt.yaml @@ -16,16 +16,19 @@ - libvirt-centos-6 - libvirt-centos-7 - libvirt-debian-8 +- libvirt-debian-9 - libvirt-fedora-25 - libvirt-fedora-26 - libvirt-fedora-rawhide - libvirt-freebsd-10 +- libvirt-freebsd-11 - autotools-syntax-check-job: parent_jobs: 'libvirt-master-build' machines: - libvirt-centos-6 - libvirt-centos-7 - libvirt-debian-8 +- libvirt-debian-9 - libvirt-fedora-25 - libvirt-fedora-26 - libvirt-fedora-rawhide -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines
FWIW, this patch alone probably makes sense to fix this particular issue.
The question is do we want to have some additional interfaces between
libvirt/qemu
that tell libvirt what CPUs/features are allowed for a specific machine version?
Halil just brough up an example. Imagine to have a QEMU binary that can emulate
an x86 and an s390x depending on the machine type. Should libvirt be able to
detect
that -cpu pentium only makes sense for the q35 machines, but not for the
s390-ccw-virtio
ones?
On 10/20/2017 04:54 PM, Christian Borntraeger wrote:
> Starting a guest with
>
> hvm
>
>
>
> on an IBM z14 results in
>
> "qemu-system-s390x: Some features requested in the CPU model are not
> available in the configuration: gs"
>
> This is because guarded storage is fenced for compat machines that did not
> have
> guarded storage support, but libvirt expands the cpu model according to the
> latest available machine.
>
> While this prevents future migration abort (by not starting the guest at all),
> not being able to start a "host-model" guest is very much unexpected. As it
> turns out, even if we would modify libvirt to not expand the cpu model to
> contain "gs" for compat machines, it cannot guarantee that a migration will
> succeed. For example if the kernel changes its features (or the user has
> nested=1 on one host but not on the other) the migration will fail
> nevertheless. So instead of fencing "gs" for machines <= 2.9 lets allow it
> for
> all machine types that support the CPU model. This will make "host-model"
> runnable all the time, while relying on the CPU model to reject invalid
> migration attempts.
>
> Suggested-by: David Hildenbrand
> Signed-off-by: Christian Borntraeger
> ---
> hw/s390x/s390-virtio-ccw.c | 8
> include/hw/s390x/s390-virtio-ccw.h | 3 ---
> target/s390x/kvm.c | 2 +-
> 3 files changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fabe4a6..ae5b01a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -414,7 +414,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void
> *data)
> s390mc->ri_allowed = true;
> s390mc->cpu_model_allowed = true;
> s390mc->css_migration_enabled = true;
> -s390mc->gs_allowed = true;
> mc->init = ccw_init;
> mc->reset = s390_machine_reset;
> mc->hot_add_cpu = s390_hot_add_cpu;
> @@ -495,12 +494,6 @@ bool cpu_model_allowed(void)
> return get_machine_class()->cpu_model_allowed;
> }
>
> -bool gs_allowed(void)
> -{
> -/* for "none" machine this results in true */
> -return get_machine_class()->gs_allowed;
> -}
> -
> static char *machine_get_loadparm(Object *obj, Error **errp)
> {
> S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> @@ -740,7 +733,6 @@ static void ccw_machine_2_9_class_options(MachineClass
> *mc)
> {
> S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>
> -s390mc->gs_allowed = false;
> ccw_machine_2_10_class_options(mc);
> SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
> s390mc->css_migration_enabled = false;
> diff --git a/include/hw/s390x/s390-virtio-ccw.h
> b/include/hw/s390x/s390-virtio-ccw.h
> index a9a90c2..ac896e3 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -40,15 +40,12 @@ typedef struct S390CcwMachineClass {
> bool ri_allowed;
> bool cpu_model_allowed;
> bool css_migration_enabled;
> -bool gs_allowed;
> } S390CcwMachineClass;
>
> /* runtime-instrumentation allowed by the machine */
> bool ri_allowed(void);
> /* cpu model allowed by the machine */
> bool cpu_model_allowed(void);
> -/* guarded-storage allowed by the machine */
> -bool gs_allowed(void);
>
> /**
> * Returns true if (vmstate based) migration of the channel subsystem
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 4c85ed8..020a7ea 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -363,7 +363,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> cap_ri = 1;
> }
> }
> -if (gs_allowed()) {
> +if (cpu_model_allowed()) {
> if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
> cap_gs = 1;
> }
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Question about the host-model CPU mode
> > I intend to put some brain-power in this too. Probably next week. > > My general impression is, that I have a at places different understanding > of how things should work compared to David. Especially when it comes > to this concept of persistent copying, and also an end-user-digestible > definition of what host-model does. (I think this with copying capabilities > from whatever xml which is subject to convoluted caching is a bit too > hard to digest for an end user not involved in the development of qemu > and libvirt). When reading the doc I was assuming that it would be a persistent copy. But it would only fix part of the issue. In the end, it specifies quite clearly what is copied. And if that is not runnable with the selected machine, bad luck. Therefore ... > > I had a conversation with Boris a couple of hours ago, and it seems, things > are pretty convoluted. > > If I understand the train of thought here (David) it can be summarized like > this: > For a certain QEMU binary no aspect of the cpu-models may depend on the > machine type. In particular, compat properties and compat handling is > not alowed to alter cpu-models (whether the available cpu models nor the > capabilities of each of these). ... I always recommended avoiding such compatibility settings in the machine. But before we had CPU models it was really all we had. No we should let go of it. We might be able to fix this one (gs) and take care of it in the future, but ... > > This we would have to make a part of the external interface. That way > one could be sure that the 'cpu capabilities' are machine-type independent > (that is, the same for all the machine types). ... the problem is once nasty stuff like zPCI comes in place. If we ever have (other?) machine dependent stuff that toggles CPU features, we can only try limit the damage. > > Or did I get this completely wrong? (Based on the answer branches my > think). Not at all. > > Halil > -- Thanks, David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH/QEMU] s390x/kvm: use cpu_model_available for guarded storage on compat machines
Starting a guest with
hvm
on an IBM z14 results in
"qemu-system-s390x: Some features requested in the CPU model are not
available in the configuration: gs"
This is because guarded storage is fenced for compat machines that did not have
guarded storage support, but libvirt expands the cpu model according to the
latest available machine.
While this prevents future migration abort (by not starting the guest at all),
not being able to start a "host-model" guest is very much unexpected. As it
turns out, even if we would modify libvirt to not expand the cpu model to
contain "gs" for compat machines, it cannot guarantee that a migration will
succeed. For example if the kernel changes its features (or the user has
nested=1 on one host but not on the other) the migration will fail
nevertheless. So instead of fencing "gs" for machines <= 2.9 lets allow it for
all machine types that support the CPU model. This will make "host-model"
runnable all the time, while relying on the CPU model to reject invalid
migration attempts.
Suggested-by: David Hildenbrand
Signed-off-by: Christian Borntraeger
---
hw/s390x/s390-virtio-ccw.c | 8
include/hw/s390x/s390-virtio-ccw.h | 3 ---
target/s390x/kvm.c | 2 +-
3 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index fabe4a6..ae5b01a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -414,7 +414,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void
*data)
s390mc->ri_allowed = true;
s390mc->cpu_model_allowed = true;
s390mc->css_migration_enabled = true;
-s390mc->gs_allowed = true;
mc->init = ccw_init;
mc->reset = s390_machine_reset;
mc->hot_add_cpu = s390_hot_add_cpu;
@@ -495,12 +494,6 @@ bool cpu_model_allowed(void)
return get_machine_class()->cpu_model_allowed;
}
-bool gs_allowed(void)
-{
-/* for "none" machine this results in true */
-return get_machine_class()->gs_allowed;
-}
-
static char *machine_get_loadparm(Object *obj, Error **errp)
{
S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -740,7 +733,6 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
{
S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
-s390mc->gs_allowed = false;
ccw_machine_2_10_class_options(mc);
SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
s390mc->css_migration_enabled = false;
diff --git a/include/hw/s390x/s390-virtio-ccw.h
b/include/hw/s390x/s390-virtio-ccw.h
index a9a90c2..ac896e3 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -40,15 +40,12 @@ typedef struct S390CcwMachineClass {
bool ri_allowed;
bool cpu_model_allowed;
bool css_migration_enabled;
-bool gs_allowed;
} S390CcwMachineClass;
/* runtime-instrumentation allowed by the machine */
bool ri_allowed(void);
/* cpu model allowed by the machine */
bool cpu_model_allowed(void);
-/* guarded-storage allowed by the machine */
-bool gs_allowed(void);
/**
* Returns true if (vmstate based) migration of the channel subsystem
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 4c85ed8..020a7ea 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -363,7 +363,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_ri = 1;
}
}
-if (gs_allowed()) {
+if (cpu_model_allowed()) {
if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
cap_gs = 1;
}
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 08/11] docs: Document user aliases
Signed-off-by: Michal Privoznik --- docs/formatdomain.html.in | 23 +++ 1 file changed, 23 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b7d7cba5a..4609e2ec2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2256,6 +2256,29 @@ + + To help users identifying devices they care about, every + device can have direct child alias element + which then has name attribute where users can + store identifier for the device. The identifier has to have + "ua-" prefix and must be unique within the domain. Additionally, the + identifier must consist only of the following characters: + [a-zA-Z0-9_-]. + Since 3.9.0 + + + ++ + + Hard drives, floppy disks, CDROMs -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list+ ++ + + ... ++
[libvirt] [PATCH v2 07/11] conf: Format alias even for inactive XMLs
We need to format alias even for inactive XMLs since that's the
way how users are going to identify their devices.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ad71e951b..38e7fee43 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5972,10 +5972,9 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
-if (info->alias &&
-!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
+
+if (info->alias)
virBufferAsprintf(buf, "\n", info->alias);
-}
if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) {
virBufferAsprintf(buf, "\n",
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/11] qemu: Parse alias from inactive XMLs
https://bugzilla.redhat.com/show_bug.cgi?id=1434451
This way users can uniquely identify devices at define time.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3b94db99f..c7c9e94da 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4176,7 +4176,8 @@ virDomainDefParserConfig
virQEMUDriverDomainDefParserConfig = {
.features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG |
VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN |
-VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS,
+VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS |
+VIR_DOMAIN_DEF_FEATURE_USER_ALIAS,
};
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/11] conf: Validate user supplied aliases
They have to be unique within the domain. As usual, backwards
compatibility takes its price. In this particular situation we
have a device that is represented twice in a domain and so is its
alias.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 148 ++-
src/conf/domain_conf.h | 5 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 +
4 files changed, 155 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40fcbc7df..ad71e951b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5457,6 +5457,145 @@ virDomainDeviceDefValidateInternal(const
virDomainDeviceDef *dev,
}
+struct virDomainDefValidateAliasesData {
+virHashTablePtr aliases;
+};
+
+
+static int
+virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def,
+ virDomainDeviceDefPtr dev,
+ virDomainDeviceInfoPtr info,
+ void *opaque)
+{
+struct virDomainDefValidateAliasesData *data = opaque;
+const char *alias = info->alias;
+
+if (!alias)
+return 0;
+
+/* Some crazy backcompat for consoles. */
+if (def->nserials && def->nconsoles &&
+def->consoles[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+def->consoles[0]->targetType ==
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL &&
+dev->type == VIR_DOMAIN_DEVICE_CHR &&
+virDomainChrEquals(def->serials[0], dev->data.chr))
+return 0;
+
+if (virHashLookup(data->aliases, alias)) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("non unique alias detected: %s"),
+ alias);
+return -1;
+}
+
+if (virHashAddEntry(data->aliases, alias, (void *) 1) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to construct table of device aliases"));
+return -1;
+}
+
+return 0;
+}
+
+
+/**
+ * virDomainDefValidateAliases:
+ *
+ * Check for uniqueness of device aliases. If @aliases is not
+ * NULL return hash table of all the aliases in it.
+ *
+ * Returns 0 on success,
+ *-1 otherwise (with error reported).
+ */
+static int
+virDomainDefValidateAliases(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
+const virDomainDef *def,
+virHashTablePtr *aliases,
+unsigned int parseFlags)
+{
+struct virDomainDefValidateAliasesData data;
+int ret = -1;
+
+/* validate configuration only in certain places */
+if (parseFlags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)
+return 0;
+
+/* We are not storing copies of aliases. Don't free them. */
+if (!(data.aliases = virHashCreate(10, NULL)))
+goto cleanup;
+
+if (virDomainDeviceInfoIterateInternal((virDomainDefPtr) def,
+
virDomainDeviceDefValidateAliasesIterator,
+ true, &data) < 0)
+goto cleanup;
+
+if (aliases) {
+*aliases = data.aliases;
+data.aliases = NULL;
+}
+
+ret = 0;
+ cleanup:
+virHashFree(data.aliases);
+return ret;
+}
+
+
+static int
+virDomainDeviceValidateAliasImpl(virDomainXMLOptionPtr xmlopt,
+ const virDomainDef *def,
+ virDomainDeviceDefPtr dev)
+{
+virHashTablePtr aliases = NULL;
+virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
+int ret = -1;
+
+if (!info || !info->alias)
+return 0;
+
+if (virDomainDefValidateAliases(xmlopt, def, &aliases, 0) < 0)
+goto cleanup;
+
+if (virHashLookup(aliases, info->alias)) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("non unique alias detected: %s"),
+ info->alias);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+
+virHashFree(aliases);
+return ret;
+}
+
+
+int
+virDomainDeviceValidateAliasForHotplug(virDomainXMLOptionPtr xmlopt,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ unsigned int flags)
+{
+virDomainDefPtr persDef = NULL;
+virDomainDefPtr liveDef = NULL;
+
+if (virDomainObjGetDefs(vm, flags, &liveDef, &persDef) < 0)
+return -1;
+
+if (persDef &&
+virDomainDeviceValidateAliasImpl(xmlopt, vm->def, dev) < 0)
+return -1;
+
+if (liveDef &&
+virDomainDeviceValidateAliasImpl(xmlopt, vm->newDef, dev) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virDomainDeviceDefValidate(const virDomainDeviceDef *dev,
const virDomainDef *def,
@@ -5668,7 +5807,9 @@ virDomainDefLifecycleActionValidate(const virDomainDef
*def)
static int
-virDomainD
[libvirt] [PATCH v2 00/11] Never ending story of user supplied aliases
v2 of: https://www.redhat.com/archives/libvir-list/2017-October/msg00790.html diff to v1: - new patch 1/11 to address Pavel's findings - reworked parsing so that the alias is set iff it follows the rules - added some tests - added news.xml entry Michal Privoznik (11): qemu_alias: Be more tolerant if alias don't follow our format conf: Parse user supplied aliases conf: Validate user supplied aliases qemuDomainABIStabilityCheck: Check for memory aliases too qemuxml2argvdata: Drop device aliases qemuhotplugtest: Load active XML conf: Format alias even for inactive XMLs docs: Document user aliases qemu: Parse alias from inactive XMLs tests: Test user set aliases for qemu news: Document user aliases docs/formatdomain.html.in | 23 +++ docs/news.xml | 9 ++ src/conf/domain_conf.c | 171 - src/conf/domain_conf.h | 6 + src/libvirt_private.syms | 1 + src/qemu/qemu_alias.c | 22 +-- src/qemu/qemu_domain.c | 18 ++- src/qemu/qemu_driver.c | 3 + tests/qemuhotplugtest.c| 3 +- .../qemuxml2argv-disk-cdrom-network-ftp.xml| 1 - .../qemuxml2argv-disk-cdrom-network-ftps.xml | 1 - .../qemuxml2argv-disk-cdrom-network-http.xml | 1 - .../qemuxml2argv-disk-cdrom-network-https.xml | 1 - .../qemuxml2argv-disk-cdrom-network-tftp.xml | 1 - .../qemuxml2argv-usb-redir-filter.xml | 1 - .../qemuxml2argv-user-aliases.args | 71 + .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 140 + tests/qemuxml2argvtest.c | 5 + .../qemuxml2xmlout-user-aliases.xml| 1 + tests/qemuxml2xmltest.c| 2 + 20 files changed, 449 insertions(+), 32 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-user-aliases.xml -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 05/11] qemuxml2argvdata: Drop device aliases
The qemuxml2argvtest expects the domain XMLs to be inactive ones. Therefore we should pass inactive XMLs. Signed-off-by: Michal Privoznik --- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml | 1 - tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml | 1 - tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml | 1 - tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml | 1 - tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml | 1 - tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml | 1 - 6 files changed, 6 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml index df41e5832..b4e331160 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftp.xml @@ -26,7 +26,6 @@ - diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml index 57049a266..4e6ca1bad 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-ftps.xml @@ -26,7 +26,6 @@ - diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml index 8a1286cb1..0eed65672 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.xml @@ -26,7 +26,6 @@ - diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml index 41b5a89b6..cd92fe44a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-https.xml @@ -26,7 +26,6 @@ - diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml index e2cb4a006..1c3b185b0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.xml @@ -26,7 +26,6 @@ - diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml index 32fb890a1..0bc15f0df 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir-filter.xml @@ -33,7 +33,6 @@ - -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 10/11] tests: Test user set aliases for qemu
Signed-off-by: Michal Privoznik --- .../qemuxml2argv-user-aliases.args | 71 +++ .../qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 140 + tests/qemuxml2argvtest.c | 5 + .../qemuxml2xmlout-user-aliases.xml| 1 + tests/qemuxml2xmltest.c| 2 + 5 files changed, 219 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml create mode 12 tests/qemuxml2xmloutdata/qemuxml2xmlout-user-aliases.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.args b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.args new file mode 100644 index 0..62fbd567b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.args @@ -0,0 +1,71 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name gentoo \ +-S \ +-M pc-i440fx-1.4 \ +-m 4096 \ +-smp 4,sockets=4,cores=1,threads=1 \ +-object memory-backend-file,id=ram-node0,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu/-1-gentoo,size=1073741824 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-object memory-backend-file,id=ram-node1,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu/-1-gentoo,size=1073741824 \ +-numa node,nodeid=1,cpus=1,memdev=ram-node1 \ +-object memory-backend-file,id=ram-node2,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu/-1-gentoo,size=1073741824 \ +-numa node,nodeid=2,cpus=2,memdev=ram-node2 \ +-object memory-backend-file,id=ram-node3,prealloc=yes,\ +mem-path=/dev/hugepages1G/libvirt/qemu/-1-gentoo,size=1073741824 \ +-numa node,nodeid=3,cpus=3,memdev=ram-node3 \ +-uuid a75aca4b-a02f-2bcb-4a91-c93cd848c34b \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-gentoo/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-global PIIX4_PM.disable_s3=0 \ +-global PIIX4_PM.disable_s4=0 \ +-boot cd \ +-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x8 \ +-usb \ +-drive file=/var/lib/libvirt/images/fd.img,format=raw,if=none,\ +id=drive-ua-myDisk1,cache=none \ +-global isa-fdc.driveA=drive-ua-myDisk1 \ +-drive file=/var/lib/libvirt/images/gentoo.qcow2,format=qcow2,if=none,\ +id=drive-ua-myDisk2 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-ua-myDisk2,id=ua-myDisk2 \ +-drive file=/var/lib/libvirt/images/OtherDemo.img,format=qcow2,if=none,\ +id=drive-ua-myEncryptedDisk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-ua-myEncryptedDisk1,\ +id=ua-myEncryptedDisk1 \ +-drive file=/home/zippy/tmp/install-amd64-minimal-20140619.iso,format=raw,\ +if=none,media=cdrom,id=drive-ua-WhatAnAwesomeCDROM,readonly=on,cache=none \ +-device ide-drive,bus=ua-DoesAnybodyStillUseIDE.1,unit=0,\ +drive=drive-ua-WhatAnAwesomeCDROM,id=ua-WhatAnAwesomeCDROM \ +-device virtio-net-pci,vlan=0,id=ua-CheckoutThisNIC,mac=52:54:00:d6:c0:0b,\ +bus=pci.0,addr=0x3 \ +-net tap,fd=3,vlan=0,name=hostua-CheckoutThisNIC \ +-device rtl8139,vlan=1,id=ua-WeCanAlsoDoServerMode,mac=52:54:00:22:c9:42,\ +bus=pci.0,addr=0x9 \ +-net socket,listen=127.0.0.1:1234,vlan=1,name=hostua-WeCanAlsoDoServerMode \ +-device rtl8139,vlan=2,id=ua-AndAlsoClientMode,mac=52:54:00:8c:b1:f8,bus=pci.0,\ +addr=0xa \ +-net socket,connect=127.0.0.1:1234,vlan=2,name=hostua-AndAlsoClientMode \ +-chardev pty,id=charserial0 \ +-device isa-serial,chardev=charserial0,id=serial0 \ +-chardev pty,id=charserial1 \ +-device isa-serial,chardev=charserial1,id=serial1 \ +-chardev socket,id=charchannel0,\ +path=/var/lib/libvirt/qemu/channel/target/gentoo.org.qemu.guest_agent.0,server,\ +nowait \ +-device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\ +id=channel0,name=org.qemu.guest_agent.0 \ +-vnc 127.0.0.1:0 \ +-vga cirrus \ +-device intel-hda,id=sound0,bus=pci.0,addr=0x4 \ +-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml new file mode 100644 index 0..d1cb8fea6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml @@ -0,0 +1,140 @@ + + gentoo + a75aca4b-a02f-2bcb-4a91-c93cd848c34b + 4194304 + 4194304 + + + + + + 4 + +hvm + + + + + + + + + + + + + + + + + + destroy + restart + restart + + + + + +/usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
[libvirt] [PATCH v2 11/11] news: Document user aliases
Signed-off-by: Michal Privoznik --- docs/news.xml | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index ff36c800a..989b82aa5 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -40,6 +40,15 @@ Add capability to allow hot (un)plug of a domain watchdog device + + + Allow users to set device aliases + + + Users can set aliases to domain devices and thus identify them + easily. + + -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/11] qemuDomainABIStabilityCheck: Check for memory aliases too
Since we will be allowing users to set device aliases and memory
devices are fragile when it comes to aliases we have to make sure
they won't change during migration. Other devices should be fine.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ece8ee7dd..3b94db99f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6428,6 +6428,8 @@ static bool
qemuDomainABIStabilityCheck(const virDomainDef *src,
const virDomainDef *dst)
{
+size_t i;
+
if (src->mem.source != dst->mem.source) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target memoryBacking source '%s' doesn't "
@@ -6437,6 +6439,19 @@ qemuDomainABIStabilityCheck(const virDomainDef *src,
return false;
}
+for (i = 0; i < src->nmems; i++) {
+const char *srcAlias = src->mems[i]->info.alias;
+const char *dstAlias = dst->mems[i]->info.alias;
+
+if (STRNEQ_NULLABLE(srcAlias, dstAlias)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Target memory device alias '%s' doesn't "
+ "match source alias '%s'"),
+ NULLSTR(srcAlias), NULLSTR(dstAlias));
+return false;
+}
+}
+
return true;
}
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 02/11] conf: Parse user supplied aliases
If driver that is calling the parse supports user supplied
aliases, they can be parsed even for inactive XMLs. However, to
avoid any clashes with aliases that libvirt generates, the user
ones have to have "ua-" prefix.
Note, that some drivers don't have notion of device aliases at
all. Also, in order to support user supplied aliases some extra
checks need to be done (e.g. during hotplug). Therefore we can't
just enable this feature for all the drivers. Thus we need a flag
that drivers set to tell parsing code that they can handle user
supplied device aliases.
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 18 +++---
src/conf/domain_conf.h | 1 +
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ce9b4ee7f..40fcbc7df 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6454,6 +6454,10 @@ virDomainDeviceAddressParseXML(xmlNodePtr address,
}
+#define USER_ALIAS_PREFIX "ua-"
+#define USER_ALIAS_CHARS \
+"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-"
+
/* Parse the XML definition for a device address
* @param node XML nodeset to parse for device address definition
*/
@@ -6472,6 +6476,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
xmlNodePtr rom = NULL;
char *type = NULL;
char *rombar = NULL;
+char *aliasStr = NULL;
int ret = -1;
virDomainDeviceInfoClear(info);
@@ -6480,7 +6485,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
if (alias == NULL &&
-!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
virXMLNodeNameEqual(cur, "alias")) {
alias = cur;
} else if (address == NULL &&
@@ -6502,8 +6506,15 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
cur = cur->next;
}
-if (alias)
-info->alias = virXMLPropString(alias, "name");
+if (alias) {
+aliasStr = virXMLPropString(alias, "name");
+
+if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) ||
+(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_USER_ALIAS &&
+ STRPREFIX(aliasStr, USER_ALIAS_PREFIX) &&
+ strspn(aliasStr, USER_ALIAS_CHARS) == strlen(aliasStr)))
+VIR_STEAL_PTR(info->alias, aliasStr);
+}
if (master) {
info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
@@ -6537,6 +6548,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
virDomainDeviceInfoClear(info);
VIR_FREE(type);
VIR_FREE(rombar);
+VIR_FREE(aliasStr);
return ret;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f251f390b..1a3574aa7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2488,6 +2488,7 @@ typedef enum {
VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN = (1 << 2),
VIR_DOMAIN_DEF_FEATURE_NAME_SLASH = (1 << 3),
VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4),
+VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),
} virDomainDefFeatures;
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/11] qemu_alias: Be more tolerant if alias don't follow our format
When assigning alias to a device we usually iterate over other
devices of its kind trying to find next index. We do this by
stripping down the prefix and then parsing number at the end,
Usually, if the prefix doesn't match the one we are expecting, we
just continue with next iteration. Except for couple of
functions: qemuGetNextChrDevIndex(),
qemuAssignDeviceRedirdevAlias() and qemuAssignDeviceShmemAlias().
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_alias.c | 22 ++
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index d06a3d63e..37fe2aa80 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -73,11 +73,8 @@ qemuGetNextChrDevIndex(virDomainDefPtr def,
ssize_t thisidx;
if (((thisidx = qemuDomainDeviceAliasIndex(&arrPtr[i]->info, prefix))
< 0) &&
(prefix2 &&
- (thisidx = qemuDomainDeviceAliasIndex(&arrPtr[i]->info, prefix2))
< 0)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to determine device index for character
device"));
-return -1;
-}
+ (thisidx = qemuDomainDeviceAliasIndex(&arrPtr[i]->info, prefix2))
< 0))
+continue;
if (thisidx >= idx)
idx = thisidx + 1;
}
@@ -391,11 +388,8 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def,
idx = 0;
for (i = 0; i < def->nredirdevs; i++) {
int thisidx;
-if ((thisidx =
qemuDomainDeviceAliasIndex(&def->redirdevs[i]->info, "redir")) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to determine device index for
redirected device"));
-return -1;
-}
+if ((thisidx =
qemuDomainDeviceAliasIndex(&def->redirdevs[i]->info, "redir")) < 0)
+continue;
if (thisidx >= idx)
idx = thisidx + 1;
}
@@ -491,12 +485,8 @@ qemuAssignDeviceShmemAlias(virDomainDefPtr def,
int thisidx;
if ((thisidx = qemuDomainDeviceAliasIndex(&def->shmems[i]->info,
- "shmem")) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unable to determine device index "
- "for shmem device"));
-return -1;
-}
+ "shmem")) < 0)
+continue;
if (thisidx >= idx)
idx = thisidx + 1;
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/11] qemuhotplugtest: Load active XML
The point of this test is to load live XML and test hotplug. But
even though the XMLs we are parsing are live, the parsing is done
with VIR_DOMAIN_DEF_PARSE_INACTIVE flag.
Signed-off-by: Michal Privoznik
---
tests/qemuhotplugtest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index df28a7fbd..8d77c0056 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -62,6 +62,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = NULL;
+const unsigned int parseFlags = 0;
if (!(*vm = virDomainObjNew(xmlopt)))
goto cleanup;
@@ -87,7 +88,7 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
driver.caps,
driver.xmlopt,
NULL,
- VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+ parseFlags)))
goto cleanup;
if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps,
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Libvirt xl to xml converter only picks up first occurrence of an option
Hi Jim I discovered that libvirt's native config file to xml converter for libxl only pick up the first occurrence of an option. For example in a xl cfg file: extra = "abc" ... extra = "def" xl will pick up "def" because that shows up later and takes precedence, while the converter picks up "abc". I think this is a bug in the converter and should be fixed. Thanks Wei. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-glib] [PATCH] spec: Enable unit tests during build
Hi, libvirt-glib has unit tests which can be evaluated as a part of build of RPM. It is generally recommended to run these tests so that some problems can be cought soon enough before the package gets to the users. Please, enable these tests for libvirt-glib. I've done scratch-build of this in koji and the build passes as well as the tests. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: Enable unit tests during build
From: Marek Kasik
Enable unit tests so that we can catch some problems soon enough
before the package gets to the users.
https://bugzilla.redhat.com/show_bug.cgi?id=1502639
---
libvirt-glib.spec.in | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libvirt-glib.spec.in b/libvirt-glib.spec.in
index a1ca11f..7191862 100644
--- a/libvirt-glib.spec.in
+++ b/libvirt-glib.spec.in
@@ -116,6 +116,9 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/libvirt-gobject-1.0.la
%find_lang %{name}
+%check
+%__make %{?_smp_mflags} check
+
%clean
rm -rf $RPM_BUILD_ROOT
--
2.14.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: duplicate interface name instead of MAC provided to lookup the interface
On Fri, Oct 20, 2017 at 04:25:13PM +0200, Andrea Bolognani wrote:
> On Fri, 2017-10-20 at 16:07 +0200, Pavel Hrdina wrote:
> > Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129.
> > Found by running libvirt-perl tests.
> >
> > Signed-off-by: Pavel Hrdina
> > ---
> > src/conf/virinterfaceobj.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
> > index a6814a6aee..21d76e7507 100644
> > --- a/src/conf/virinterfaceobj.c
> > +++ b/src/conf/virinterfaceobj.c
> > @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload,
> > virObjectLock(obj);
> >
> > if (STRCASEEQ(obj->def->mac, data->matchStr)) {
> > -if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) {
> > +if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) {
> > data->error = true;
> > goto cleanup;
> > }
>
> Reviewed-by: Andrea Bolognani
Thanks
> As an aside, I think 'macs' is a pretty bad name for the array, since
> we're not storing MAC addresses but rather interface names, so using
> either 'matches' or 'names' would be much better IMHO. Do you want me
> to send a follow-up patch performing the rename?
That would be good idea, feel free to send it and probably push it as
trivial.
Pavel
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] libvirt/QEMU/SEV interaction
On 10/18/17 8:35 PM, Michael S. Tsirkin wrote: On Wed, Oct 18, 2017 at 08:18:48PM +0100, Dr. David Alan Gilbert wrote: * Michael S. Tsirkin (m...@redhat.com) wrote: On Fri, Sep 08, 2017 at 10:48:10AM -0500, Brijesh Singh wrote: > 11. GO verifies the measurement and if measurement matches then it may > give a secret blob -- which must be injected into the guest before > libvirt starts the VM. If verification failed, GO will request cloud > provider to destroy the VM. I realised I'm missing something here: how does GO limit the secret to the specific VM? For example, what prevents hypervisor from launching two VMs with the same GO's DH, getting measurement from 1st one but injecting the secret into the second one? Isn't that the 'trusted channel nonce currently associated with the guest' in the guest context? Dave Let me try to clarify the question. I understand that sometimes a key is shared between VMs. If this is allowed, it seems that a hypervisor can run any number of VMs with the same key. An unauthorised VM will not get a measurement that guest owner authorizes, but can the hypervisor get secret intended for an authorized VM and then inject it into an unauthorized one sharing the same key? Michael, Yes, that's possible. This is why we recommend against a guest owner authorizing key sharing. There's no way for the guest owner to control what other guests the HV might install using the same key and mapping the same memory to. So a security-conscious customer, especially in a cloud environment, should never enable key sharing. Richard Thanks, -- MST -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: duplicate interface name instead of MAC provided to lookup the interface
On Fri, 2017-10-20 at 16:07 +0200, Pavel Hrdina wrote:
> Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129.
> Found by running libvirt-perl tests.
>
> Signed-off-by: Pavel Hrdina
> ---
> src/conf/virinterfaceobj.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
> index a6814a6aee..21d76e7507 100644
> --- a/src/conf/virinterfaceobj.c
> +++ b/src/conf/virinterfaceobj.c
> @@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload,
> virObjectLock(obj);
>
> if (STRCASEEQ(obj->def->mac, data->matchStr)) {
> -if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) {
> +if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) {
> data->error = true;
> goto cleanup;
> }
Reviewed-by: Andrea Bolognani
As an aside, I think 'macs' is a pretty bad name for the array, since
we're not storing MAC addresses but rather interface names, so using
either 'matches' or 'names' would be much better IMHO. Do you want me
to send a follow-up patch performing the rename?
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [perl PATCH 0/2] Add missing bindings
On Fri, Oct 20, 2017 at 04:05:38PM +0200, Pavel Hrdina wrote: > Pavel Hrdina (2): > Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant > Add set_lifecycle_action > > Changes| 3 ++- > Virt.xs| 24 > lib/Sys/Virt/Domain.pm | 60 > ++ > 3 files changed, 86 insertions(+), 1 deletion(-) Reviewed-by: Daniel P. Berrange 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] Question about the host-model CPU mode
On 10/20/2017 04:06 PM, David Hildenbrand wrote:
> On 20.10.2017 16:02, Christian Borntraeger wrote:
>>
>>
>> On 10/20/2017 03:51 PM, David Hildenbrand wrote:
>> [...]
The problem goes much further.
A fresh guest with
hvm
does not start. No migration from an older system is necessary.
>>>
>>> Yes, as stated in the documentation "copying host CPU definition from
>>> capabilities XML" this can not work. And it works just as documented.
>>> Not saying this is a nice thing :)
>>>
>>> I think we should try to fix gs_allowed (if possible) and avoid
>>> something like that in the future. This would avoid other complexity
>>> involved when suddenly having X host models.
>>
>> Maybe this one is really a proper fix. It will allow the guest to start
>> and on migration the cpu model will complain if the target cannot provide gs.
>> Similar things can happen if - for example - the host kernel lacks some
>> features.
>
> Right, just what I had in mind.
>
>>
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 77169bb..97a08fa 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void
>> *data)
>> s390mc->ri_allowed = true;
>> s390mc->cpu_model_allowed = true;
>> s390mc->css_migration_enabled = true;
>> -s390mc->gs_allowed = true;
>> mc->init = ccw_init;
>> mc->reset = s390_machine_reset;
>> mc->hot_add_cpu = s390_hot_add_cpu;
>> @@ -509,12 +508,6 @@ bool cpu_model_allowed(void)
>> return get_machine_class()->cpu_model_allowed;
>> }
>>
>> -bool gs_allowed(void)
>> -{
>> -/* for "none" machine this results in true */
>> -return get_machine_class()->gs_allowed;
>> -}
>> -
>> static char *machine_get_loadparm(Object *obj, Error **errp)
>> {
>> S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> @@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass
>> *mc)
>> {
>> S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>>
>> -s390mc->gs_allowed = false;
>> ccw_machine_2_10_class_options(mc);
>> SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
>> s390mc->css_migration_enabled = false;
>> diff --git a/include/hw/s390x/s390-virtio-ccw.h
>> b/include/hw/s390x/s390-virtio-ccw.h
>> index a9a90c2..1de53b0 100644
>> --- a/include/hw/s390x/s390-virtio-ccw.h
>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>> @@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass {
>> bool ri_allowed;
>> bool cpu_model_allowed;
>> bool css_migration_enabled;
>> -bool gs_allowed;
>> } S390CcwMachineClass;
>>
>> /* runtime-instrumentation allowed by the machine */
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index a0d5052..3f13fc2 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>> cap_ri = 1;
>> }
>> }
>> -if (gs_allowed()) {
>> +if (cpu_model_allowed()) {
>> if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
>> cap_gs = 1;
>> }
>>
>
> And the last hunk makes sure we keep same handling for machines without
> CPU model support and therefore no way to mask support. For all recent
> machines, we expect CPU model checks to be in place.
>
> Will have to think about this a bit more. Will you send this as a proper
> patch?
After thinking about it :-)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: duplicate interface name instead of MAC provided to lookup the interface
Introduced by 6094d6ec7fc9ea3e28c18c880b76858f06a8b129.
Found by running libvirt-perl tests.
Signed-off-by: Pavel Hrdina
---
src/conf/virinterfaceobj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index a6814a6aee..21d76e7507 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -182,7 +182,7 @@ virInterfaceObjListFindByMACStringCb(void *payload,
virObjectLock(obj);
if (STRCASEEQ(obj->def->mac, data->matchStr)) {
-if (VIR_STRDUP(data->macs[data->nmacs], data->matchStr) < 0) {
+if (VIR_STRDUP(data->macs[data->nmacs], obj->def->name) < 0) {
data->error = true;
goto cleanup;
}
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 20.10.2017 16:02, Christian Borntraeger wrote:
>
>
> On 10/20/2017 03:51 PM, David Hildenbrand wrote:
> [...]
>>> The problem goes much further.
>>> A fresh guest with
>>>
>>>
>>> hvm
>>>
>>>
>>>
>>> does not start. No migration from an older system is necessary.
>>>
>>
>> Yes, as stated in the documentation "copying host CPU definition from
>> capabilities XML" this can not work. And it works just as documented.
>> Not saying this is a nice thing :)
>>
>> I think we should try to fix gs_allowed (if possible) and avoid
>> something like that in the future. This would avoid other complexity
>> involved when suddenly having X host models.
>
> Maybe this one is really a proper fix. It will allow the guest to start
> and on migration the cpu model will complain if the target cannot provide gs.
> Similar things can happen if - for example - the host kernel lacks some
> features.
Right, just what I had in mind.
>
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 77169bb..97a08fa 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void
> *data)
> s390mc->ri_allowed = true;
> s390mc->cpu_model_allowed = true;
> s390mc->css_migration_enabled = true;
> -s390mc->gs_allowed = true;
> mc->init = ccw_init;
> mc->reset = s390_machine_reset;
> mc->hot_add_cpu = s390_hot_add_cpu;
> @@ -509,12 +508,6 @@ bool cpu_model_allowed(void)
> return get_machine_class()->cpu_model_allowed;
> }
>
> -bool gs_allowed(void)
> -{
> -/* for "none" machine this results in true */
> -return get_machine_class()->gs_allowed;
> -}
> -
> static char *machine_get_loadparm(Object *obj, Error **errp)
> {
> S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> @@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass
> *mc)
> {
> S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
>
> -s390mc->gs_allowed = false;
> ccw_machine_2_10_class_options(mc);
> SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
> s390mc->css_migration_enabled = false;
> diff --git a/include/hw/s390x/s390-virtio-ccw.h
> b/include/hw/s390x/s390-virtio-ccw.h
> index a9a90c2..1de53b0 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass {
> bool ri_allowed;
> bool cpu_model_allowed;
> bool css_migration_enabled;
> -bool gs_allowed;
> } S390CcwMachineClass;
>
> /* runtime-instrumentation allowed by the machine */
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a0d5052..3f13fc2 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> cap_ri = 1;
> }
> }
> -if (gs_allowed()) {
> +if (cpu_model_allowed()) {
> if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
> cap_gs = 1;
> }
>
And the last hunk makes sure we keep same handling for machines without
CPU model support and therefore no way to mask support. For all recent
machines, we expect CPU model checks to be in place.
Will have to think about this a bit more. Will you send this as a proper
patch?
--
Thanks,
David
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 2/2] Add set_lifecycle_action
Signed-off-by: Pavel Hrdina --- Changes| 1 + Virt.xs| 23 + lib/Sys/Virt/Domain.pm | 56 ++ 3 files changed, 80 insertions(+) diff --git a/Changes b/Changes index 06f66c8..ebc6548 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,7 @@ Revision history for perl module Sys::Virt 3.9.0 2017-11-00 - Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant + - Add set_lifecycle_action 3.8.0 2017-10-04 diff --git a/Virt.xs b/Virt.xs index eb8465f..c123769 100644 --- a/Virt.xs +++ b/Virt.xs @@ -6310,6 +6310,16 @@ set_block_threshold(dom, dev, thresholdsv, flags=0) _croak_error(); void +set_lifecycle_action(dom, type, action, flags=0) + virDomainPtr dom; + unsigned int type; + unsigned int action; + unsigned int flags; +PPCODE: + if (virDomainSetLifecycleAction(dom, type, action, flags) < 0) + _croak_error(); + +void destroy(dom_rv, flags=0) SV *dom_rv; unsigned int flags; @@ -9091,6 +9101,19 @@ BOOT: REGISTER_CONSTANT(VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED, REVERT_PAUSED); REGISTER_CONSTANT(VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, REVERT_FORCE); + stash = gv_stashpv( "Sys::Virt::Lifecycle", TRUE ); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_POWEROFF, LIFECYCLE_POWEROFF); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_REBOOT, LIFECYCLE_REBOOT); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_CRASH, LIFECYCLE_CRASH); + + stash = gv_stashpv( "Sys::Virt::LifecycleAction", TRUE ); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY, LIFECYCLE_ACTION_DESTROY); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_ACTION_RESTART, LIFECYCLE_ACTION_RESTART); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME, LIFECYCLE_ACTION_RESTART_RENAME); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE, LIFECYCLE_ACTION_PRESERVE); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY, LIFECYCLE_ACTION_COREDUMP_DESTROY); + REGISTER_CONSTANT(VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART, LIFECYCLE_ACTION_COREDUMP_RESTART); + stash = gv_stashpv( "Sys::Virt::StoragePool", TRUE ); REGISTER_CONSTANT(VIR_STORAGE_POOL_INACTIVE, STATE_INACTIVE); REGISTER_CONSTANT(VIR_STORAGE_POOL_BUILDING, STATE_BUILDING); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 273e6e6..38b9c9b 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1999,6 +1999,11 @@ level. The threshold level is unset once the event fires. The event might not be delivered at all if libvirtd was not running at the moment when the threshold was reached. +=item $dom->set_lifecycle_action($type, $action, $flags=0) + +Changes the actions of lifecycle events for domain represented as +$action in the domain XML. + =back =head1 CONSTANTS @@ -4544,6 +4549,57 @@ The I/O threads pinning =back +=head2 DOMAIN LIFECYCLE CONSTANTS + +The following constants are useful when setting action for +lifecycle events. + +=over 4 + +=item Sys::Virt::Domain::LIFECYCLE_POWEROFF + +The poweroff lifecycle event type + +=item Sys::Virt::Domain::LIFECYCLE_REBOOT + +The reboot lifecycle event type + +=item Sys::Virt::Domain::LIFECYCLE_CRASH + +The crash lifecycle event type + +=back + +=head2 DOMAIN LIFECYCLE ACTION CONSTANTS + +=over 4 + +=item Sys::Virt::Domain::LIFECYCLE_ACTION_DESTROY + +The destroy lifecycle action + +=item Sys::Virt::Domain::LIFECYCLE_ACTION_RESTART + +The restart lifecycle action + +=item Sys::Virt::Domain::LIFECYCLE_ACTION_RESTART_RENAME + +The restart-rename lifecycle action + +=item Sys::Virt::Domain::LIFECYCLE_ACTION_PRESERVE + +The preserve lifecycle action + +=item Sys::Virt::Domain::LIFECYCLE_ACTION_COREDUMP_DESTROY + +The coredump-destroy lifecycle action + +=item Sys::Virt::Domain::LIFECYCLE_ACTION_COREDUMP_RESTART + +The coredump-restart lifecycle action + +=back + =head1 AUTHORS Daniel P. Berrange -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 1/2] Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant
Signed-off-by: Pavel Hrdina --- Changes| 2 +- Virt.xs| 1 + lib/Sys/Virt/Domain.pm | 4 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Changes b/Changes index 0cf810e..06f66c8 100644 --- a/Changes +++ b/Changes @@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt 3.9.0 2017-11-00 - - XXX + - Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant 3.8.0 2017-10-04 diff --git a/Virt.xs b/Virt.xs index 9243b7f..eb8465f 100644 --- a/Virt.xs +++ b/Virt.xs @@ -8666,6 +8666,7 @@ BOOT: REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_TOTAL, JOB_MEMORY_TOTAL); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_BPS, JOB_MEMORY_BPS); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_DIRTY_RATE, JOB_MEMORY_DIRTY_RATE); + REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE, JOB_MEMORY_PAGE_SIZE); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_MEMORY_ITERATION, JOB_MEMORY_ITERATION); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_SETUP_TIME, JOB_SETUP_TIME); REGISTER_CONSTANT_STR(VIR_DOMAIN_JOB_TIME_ELAPSED, JOB_TIME_ELAPSED); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 59b0215..273e6e6 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1566,6 +1566,10 @@ The bytes per second transferred The number of memory pages dirtied per second +=item Sys::Virt::Domain::JOB_MEMORY_PAGE_SIZE + +The memory page size in bytes + =item Sys::Virt::Domain::JOB_MEMORY_ITERATION The total number of iterations over guest memory -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 0/2] Add missing bindings
Pavel Hrdina (2): Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant Add set_lifecycle_action Changes| 3 ++- Virt.xs| 24 lib/Sys/Virt/Domain.pm | 60 ++ 3 files changed, 86 insertions(+), 1 deletion(-) -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 10/20/2017 03:51 PM, David Hildenbrand wrote:
[...]
>> The problem goes much further.
>> A fresh guest with
>>
>>
>> hvm
>>
>>
>>
>> does not start. No migration from an older system is necessary.
>>
>
> Yes, as stated in the documentation "copying host CPU definition from
> capabilities XML" this can not work. And it works just as documented.
> Not saying this is a nice thing :)
>
> I think we should try to fix gs_allowed (if possible) and avoid
> something like that in the future. This would avoid other complexity
> involved when suddenly having X host models.
Maybe this one is really a proper fix. It will allow the guest to start
and on migration the cpu model will complain if the target cannot provide gs.
Similar things can happen if - for example - the host kernel lacks some
features.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 77169bb..97a08fa 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -430,7 +430,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void
*data)
s390mc->ri_allowed = true;
s390mc->cpu_model_allowed = true;
s390mc->css_migration_enabled = true;
-s390mc->gs_allowed = true;
mc->init = ccw_init;
mc->reset = s390_machine_reset;
mc->hot_add_cpu = s390_hot_add_cpu;
@@ -509,12 +508,6 @@ bool cpu_model_allowed(void)
return get_machine_class()->cpu_model_allowed;
}
-bool gs_allowed(void)
-{
-/* for "none" machine this results in true */
-return get_machine_class()->gs_allowed;
-}
-
static char *machine_get_loadparm(Object *obj, Error **errp)
{
S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -757,7 +750,6 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
{
S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
-s390mc->gs_allowed = false;
ccw_machine_2_10_class_options(mc);
SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
s390mc->css_migration_enabled = false;
diff --git a/include/hw/s390x/s390-virtio-ccw.h
b/include/hw/s390x/s390-virtio-ccw.h
index a9a90c2..1de53b0 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -40,7 +40,6 @@ typedef struct S390CcwMachineClass {
bool ri_allowed;
bool cpu_model_allowed;
bool css_migration_enabled;
-bool gs_allowed;
} S390CcwMachineClass;
/* runtime-instrumentation allowed by the machine */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a0d5052..3f13fc2 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -362,7 +362,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_ri = 1;
}
}
-if (gs_allowed()) {
+if (cpu_model_allowed()) {
if (kvm_vm_enable_cap(s, KVM_CAP_S390_GS, 0) == 0) {
cap_gs = 1;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 20.10.2017 15:49, Christian Borntraeger wrote: > > > On 10/20/2017 03:43 PM, David Hildenbrand wrote: >> On 20.10.2017 15:36, Christian Borntraeger wrote: >>> >>> >>> On 10/20/2017 03:16 PM, David Hildenbrand wrote: > Hi all, > > we recently encountered the problem that the 'host-model' [1] has to be > related to the machine type of a domain. We have following problem: > >Let's assume we've a z13 system with a QEMU 2.9 and we define a >domain using the default s390-virtio-ccw machine together with the >host-model CPU mode [1]. The definition will have the machine >expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >in the domain definition. In a next step we upgrade to QEMU 2.10 >(first version to recognize z14). Everything is still fine, even >though the machine runs in 2.9 compatibility mode. Finally we >upgrade to a z14. As a consequence it is not possible to start the >domain anymore as the machine type doesn't support our CPU host >model (which is expanded at start time of the domain). Actually, what is the cause of that problem? I assume it is the gs feature (gs_allowed)? We should really avoid such things (..._allowed) for CPU model features in the future and clue all new such stuff to cpumodel_allowed. >>> >>> Yes, starting a guest with >>> >>> hvm >>> >>> >>> >>> results in >>> >>> qemu-system-s390x: Some features requested in the CPU model are not >>> available in the configuration: gs >>> >>> Tying it to cpumodel_allowed would not help, migration-wise though. >>> libvirt would still transform >>> >>> >>> hvm >>> >>> >> >> My point was, that the host model would have to be copied and _remain_ >> there when s390-ccw-virtio was expanded to s390-ccw-virtio-2.9. >> >> So really replacing by the model z13-base >> This would at least fix this issue. Just like s390-ccw-virtio get's >> replaced and remains thats way. >> >> But this might for sure have other problems. > > The problem goes much further. > A fresh guest with > > > hvm > > > > does not start. No migration from an older system is necessary. > Yes, as stated in the documentation "copying host CPU definition from capabilities XML" this can not work. And it works just as documented. Not saying this is a nice thing :) I think we should try to fix gs_allowed (if possible) and avoid something like that in the future. This would avoid other complexity involved when suddenly having X host models. -- Thanks, David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
Separate it so that it deals only with single virStorageSource, so that
it can later be reused for full backing chain support.
Two aliases are passed since authentication is more relevant to the
'storage backend' whereas encryption is more relevant to the protocol
layer. When using node names, the aliases will be different.
---
src/qemu/qemu_domain.c | 49 +++--
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24ed61bc2..4a2ba1761 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1368,27 +1368,19 @@ qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr
src)
}
-/* qemuDomainSecretDiskPrepare:
- * @conn: Pointer to connection
- * @priv: pointer to domain private object
- * @disk: Pointer to a disk definition
- *
- * For the right disk, generate the qemuDomainSecretInfo structure.
- *
- * Returns 0 on success, -1 on failure
- */
-int
-qemuDomainSecretDiskPrepare(virConnectPtr conn,
-qemuDomainObjPrivatePtr priv,
-virDomainDiskDefPtr disk)
+static int
+qemuDomainSecretStorageSourcePrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ virStorageSourcePtr src,
+ const char *authalias,
+ const char *encalias)
{
-virStorageSourcePtr src = disk->src;
qemuDomainStorageSourcePrivatePtr srcPriv;
-if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew()))
+if (!(src->privateData = qemuDomainStorageSourcePrivateNew()))
return -1;
-srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
if (qemuDomainSecretDiskCapable(src)) {
virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
@@ -1397,7 +1389,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
usageType = VIR_SECRET_USAGE_TYPE_CEPH;
if (!(srcPriv->secinfo =
- qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
+ qemuDomainSecretInfoNew(conn, priv, authalias,
usageType, src->auth->username,
&src->auth->seclookupdef, false)))
return -1;
@@ -1405,7 +1397,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
if (qemuDomainDiskHasEncryptionSecret(src)) {
if (!(srcPriv->encinfo =
- qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
+ qemuDomainSecretInfoNew(conn, priv, encalias,
VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
&src->encryption->secrets[0]->seclookupdef,
true)))
@@ -1416,6 +1408,27 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
}
+/* qemuDomainSecretDiskPrepare:
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @disk: Pointer to a disk definition
+ *
+ * For the right disk, generate the qemuDomainSecretInfo structure.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+
+int
+qemuDomainSecretDiskPrepare(virConnectPtr conn,
+qemuDomainObjPrivatePtr priv,
+virDomainDiskDefPtr disk)
+{
+return qemuDomainSecretStorageSourcePrepare(conn, priv, disk->src,
+disk->info.alias,
+disk->info.alias);
+}
+
+
/* qemuDomainSecretHostdevDestroy:
* @disk: Pointer to a hostdev definition
*
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 10/20/2017 03:43 PM, David Hildenbrand wrote: > On 20.10.2017 15:36, Christian Borntraeger wrote: >> >> >> On 10/20/2017 03:16 PM, David Hildenbrand wrote: >>> Hi all, we recently encountered the problem that the 'host-model' [1] has to be related to the machine type of a domain. We have following problem: Let's assume we've a z13 system with a QEMU 2.9 and we define a domain using the default s390-virtio-ccw machine together with the host-model CPU mode [1]. The definition will have the machine expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode in the domain definition. In a next step we upgrade to QEMU 2.10 (first version to recognize z14). Everything is still fine, even though the machine runs in 2.9 compatibility mode. Finally we upgrade to a z14. As a consequence it is not possible to start the domain anymore as the machine type doesn't support our CPU host model (which is expanded at start time of the domain). >>> >>> Actually, what is the cause of that problem? I assume it is the gs >>> feature (gs_allowed)? >>> >>> We should really avoid such things (..._allowed) for CPU model features >>> in the future and clue all new such stuff to cpumodel_allowed. >> >> Yes, starting a guest with >> >> hvm >> >> >> >> results in >> >> qemu-system-s390x: Some features requested in the CPU model are not >> available in the configuration: gs >> >> Tying it to cpumodel_allowed would not help, migration-wise though. >> libvirt would still transform >> >> >> hvm >> >> > > My point was, that the host model would have to be copied and _remain_ > there when s390-ccw-virtio was expanded to s390-ccw-virtio-2.9. > > So really replacing by the model z13-base > This would at least fix this issue. Just like s390-ccw-virtio get's > replaced and remains thats way. > > But this might for sure have other problems. The problem goes much further. A fresh guest with hvm does not start. No migration from an older system is necessary. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] security: dac: Take parent security label into account
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
---
src/security/security_dac.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 244b300a9..54120890f 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -688,12 +688,14 @@ virSecurityDACRestoreFileLabel(virSecurityDACDataPtr priv,
static int
-virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
-virDomainDefPtr def,
-virStorageSourcePtr src)
+virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virStorageSourcePtr src,
+virStorageSourcePtr parent)
{
virSecurityLabelDefPtr secdef;
virSecurityDeviceLabelDefPtr disk_seclabel;
+virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
uid_t user;
gid_t group;
@@ -705,14 +707,24 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
if (secdef && !secdef->relabel)
return 0;
-disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
-SECURITY_DAC_NAME);
-if (disk_seclabel && !disk_seclabel->relabel)
-return 0;
+disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_DAC_NAME);
+if (parent)
+parent_seclabel = virStorageSourceGetSecurityLabelDef(parent,
+
SECURITY_DAC_NAME);
+
+if (disk_seclabel && (!disk_seclabel->relabel || disk_seclabel->label)) {
+if (!disk_seclabel->relabel)
+return 0;
-if (disk_seclabel && disk_seclabel->label) {
if (virParseOwnershipIds(disk_seclabel->label, &user, &group) < 0)
return -1;
+} else if (parent_seclabel &&
+ (!parent_seclabel->relabel || parent_seclabel->label)) {
+if (!parent_seclabel->relabel)
+return 0;
+
+if (virParseOwnershipIds(parent_seclabel->label, &user, &group) < 0)
+return -1;
} else {
if (virSecurityDACGetImageIds(secdef, priv, &user, &group))
return -1;
@@ -722,6 +734,14 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
}
+static int
+virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virStorageSourcePtr src)
+{
+return virSecurityDACSetImageLabelInternal(mgr, def, src, NULL);
+}
+
static int
virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
@@ -731,7 +751,7 @@ virSecurityDACSetDiskLabel(virSecurityManagerPtr mgr,
virStorageSourcePtr next;
for (next = disk->src; virStorageSourceIsBacking(next); next =
next->backingStore) {
-if (virSecurityDACSetImageLabel(mgr, def, next) < 0)
+if (virSecurityDACSetImageLabelInternal(mgr, def, next, disk->src) < 0)
return -1;
}
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] qemu: domain: Remove pointless alias check
When attaching the disks, aliases are always generated.
---
src/qemu/qemu_domain.c | 8
src/qemu/qemu_domain.h | 3 +--
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_process.c | 2 +-
4 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c689911c4..aebe24e7b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7833,7 +7833,6 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def,
/* qemuProcessPrepareDiskSourceTLS:
* @source: pointer to host interface data for disk device
- * @diskAlias: alias use for the disk device
* @cfg: driver configuration
*
* Updates host interface TLS encryption setting based on qemu.conf
@@ -7844,7 +7843,6 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def,
*/
int
qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
- const char *diskAlias,
virQEMUDriverConfigPtr cfg)
{
@@ -7863,12 +7861,6 @@ qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
}
if (src->haveTLS == VIR_TRISTATE_BOOL_YES) {
-if (!diskAlias) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("disk does not have an alias"));
-return -1;
-}
-
/* Grab the vxhsTLSx509certdir and set the verify/listen values.
* NB: tlsAlias filled in during qemuDomainGetTLSObjects. */
if (VIR_STRDUP(src->tlsCertdir, cfg->vxhsTLSx509certdir) < 0)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index a8ad59d20..6615dabf9 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -882,9 +882,8 @@ void qemuDomainPrepareChardevSource(virDomainDefPtr def,
int
qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
- const char *diskAlias,
virQEMUDriverConfigPtr cfg)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 91f7f9ed6..e4157f631 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -394,7 +394,7 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0)
goto error;
-if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) < 0)
+if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0)
goto error;
if (disk->src->haveTLS &&
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 66e81bbe5..9bbfabcde 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5301,7 +5301,7 @@ qemuProcessPrepareDomainStorage(virConnectPtr conn,
continue;
}
-if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) <
0)
+if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0)
return -1;
}
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/12] qemu: domain: Simplify using DAC permissions of top of backing chain
qemuDomainGetImageIds and qemuDomainStorageFileInit are helpful when
trying to access a virStorageSource from the qemu driver since they
figure out the correct uid and gid for the image.
When accessing members of a backing chain the permissions for the top
level would be used. To allow using specific permissions per backing
chain level but still allow inheritance from the parent of the chain we
need to add a new parameter to the image ID APIs.
---
src/qemu/qemu_domain.c | 13 ++---
src/qemu/qemu_domain.h | 3 ++-
src/qemu/qemu_driver.c | 6 +++---
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 00610edf1..24ed61bc2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5926,6 +5926,7 @@ static void
qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm,
virStorageSourcePtr src,
+ virStorageSourcePtr parentSrc,
uid_t *uid, gid_t *gid)
{
virSecurityLabelDefPtr vmlabel;
@@ -5948,6 +5949,11 @@ qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
vmlabel->label)
virParseOwnershipIds(vmlabel->label, uid, gid);
+if (parentSrc &&
+(disklabel = virStorageSourceGetSecurityLabelDef(parentSrc, "dac")) &&
+disklabel->label)
+virParseOwnershipIds(disklabel->label, uid, gid);
+
if ((disklabel = virStorageSourceGetSecurityLabelDef(src, "dac")) &&
disklabel->label)
virParseOwnershipIds(disklabel->label, uid, gid);
@@ -5957,14 +5963,15 @@ qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
int
qemuDomainStorageFileInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ virStorageSourcePtr parent)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
uid_t uid;
gid_t gid;
int ret = -1;
-qemuDomainGetImageIds(cfg, vm, src, &uid, &gid);
+qemuDomainGetImageIds(cfg, vm, src, parent, &uid, &gid);
if (virStorageFileInitAs(src, uid, gid) < 0)
goto cleanup;
@@ -6014,7 +6021,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
goto cleanup;
}
-qemuDomainGetImageIds(cfg, vm, disk->src, &uid, &gid);
+qemuDomainGetImageIds(cfg, vm, disk->src, NULL, &uid, &gid);
if (virStorageFileGetMetadata(disk->src,
uid, gid,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 39cb68b3c..a8ad59d20 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -674,7 +674,8 @@ bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virStorageSourcePtr src);
+ virStorageSourcePtr src,
+ virStorageSourcePtr parent);
char *qemuDomainStorageAlias(const char *device, int depth);
void qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d56992fbb..23692fedb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11495,7 +11495,7 @@ qemuDomainBlockPeek(virDomainPtr dom,
goto cleanup;
}
-if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0)
+if (qemuDomainStorageFileInit(driver, vm, disk->src, NULL) < 0)
goto cleanup;
if ((nread = virStorageFileRead(disk->src, offset, size, &tmpbuf)) < 0)
@@ -14418,7 +14418,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr
driver,
if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) <
0)
goto error;
-if (qemuDomainStorageFileInit(driver, vm, dd->src) < 0)
+if (qemuDomainStorageFileInit(driver, vm, dd->src, NULL) < 0)
goto error;
dd->initialized = true;
@@ -17093,7 +17093,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
goto endjob;
}
-if (qemuDomainStorageFileInit(driver, vm, mirror) < 0)
+if (qemuDomainStorageFileInit(driver, vm, mirror, NULL) < 0)
goto endjob;
if (qemuDomainBlockCopyValidateMirror(mirror, disk->dst, &reuse) < 0)
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/12] qemu: domain: skip chain detection to end of backing chain
When a user provides the backing chain, we will not need to re-detect
all the backing stores again, but should move to the end of the user
specified chain. Additionally if a user provides a full terminated chain
we should not attempt any further detection.
---
src/qemu/qemu_domain.c | 48 +++-
1 file changed, 39 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3560cdd29..5973474ca 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6030,27 +6030,57 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool report_broken)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-int ret = 0;
+virStorageSourcePtr src = disk->src;
+int ret = -1;
uid_t uid;
gid_t gid;
-if (virStorageSourceIsEmpty(disk->src))
+if (virStorageSourceIsEmpty(src)) {
+ret = 0;
goto cleanup;
+}
if (virStorageSourceHasBacking(disk->src)) {
-if (force_probe)
-virStorageSourceBackingStoreClear(disk->src);
-else
-goto cleanup;
+if (force_probe) {
+virStorageSourceBackingStoreClear(src);
+} else {
+/* skip to the end of the chain */
+while (virStorageSourceIsBacking(src)) {
+if (report_broken &&
+virStorageFileSupportsAccess(src)) {
+
+if (qemuDomainStorageFileInit(driver, vm, src, disk->src)
< 0)
+goto cleanup;
+
+if (virStorageFileAccess(src, F_OK) < 0) {
+virStorageFileReportBrokenChain(errno, src, disk->src);
+virStorageFileDeinit(src);
+goto cleanup;
+}
+
+virStorageFileDeinit(src);
+}
+src = src->backingStore;
+}
+}
}
-qemuDomainGetImageIds(cfg, vm, disk->src, NULL, &uid, &gid);
+/* We skipped to the end of the chain. Skip detection if there's the
+ * terminator. (An allocated but empty backingStore) */
+if (src->backingStore) {
+ret = 0;
+goto cleanup;
+}
+
+qemuDomainGetImageIds(cfg, vm, src, disk->src, &uid, &gid);
-if (virStorageFileGetMetadata(disk->src,
+if (virStorageFileGetMetadata(src,
uid, gid,
cfg->allowDiskFormatProbing,
report_broken) < 0)
-ret = -1;
+goto cleanup;
+
+ret = 0;
cleanup:
virObjectUnref(cfg);
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] storage: Extract error reporting for broken chains
Simplify reporting the error if backing chain is broken for further
callers by extracting it into a separate function.
---
src/storage/storage_source.c | 47 +++-
src/storage/storage_source.h | 4
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index cced5308c..4586ef4ad 100644
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -404,6 +404,38 @@ virStorageFileChown(const virStorageSource *src,
}
+/**
+ * virStorageFileReportBrokenChain:
+ *
+ * @errcode: errno when accessing @src
+ * @src: inaccessible file in the backing chain of @parent
+ * @parent: root virStorageSource being checked
+ *
+ * Reports the correct error message if @src is missing in the backing chain
+ * for @parent.
+ */
+void
+virStorageFileReportBrokenChain(int errcode,
+virStorageSourcePtr src,
+virStorageSourcePtr parent)
+{
+unsigned int access_user = src->drv->uid;
+unsigned int access_group = src->drv->gid;
+
+if (src == parent) {
+virReportSystemError(errcode,
+ _("Cannot access storage file '%s' "
+ "(as uid:%u, gid:%u)"),
+ src->path, access_user, access_group);
+} else {
+virReportSystemError(errcode,
+ _("Cannot access backing file '%s' "
+ "of storage file '%s' (as uid:%u, gid:%u)"),
+ src->path, parent->path, access_user,
access_group);
+}
+}
+
+
/* Recursive workhorse for virStorageFileGetMetadata. */
static int
virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
@@ -433,20 +465,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
return -1;
if (virStorageFileAccess(src, F_OK) < 0) {
-if (src == parent) {
-virReportSystemError(errno,
- _("Cannot access storage file '%s' "
- "(as uid:%u, gid:%u)"),
- src->path, (unsigned int)uid,
- (unsigned int)gid);
-} else {
-virReportSystemError(errno,
- _("Cannot access backing file '%s' "
- "of storage file '%s' (as uid:%u, gid:%u)"),
- src->path, parent->path,
- (unsigned int)uid, (unsigned int)gid);
-}
-
+virStorageFileReportBrokenChain(errno, src, parent);
goto cleanup;
}
diff --git a/src/storage/storage_source.h b/src/storage/storage_source.h
index 320ea3cab..0640c138e 100644
--- a/src/storage/storage_source.h
+++ b/src/storage/storage_source.h
@@ -52,4 +52,8 @@ int virStorageFileGetMetadata(virStorageSourcePtr src,
char *virStorageFileGetBackingStoreStr(virStorageSourcePtr src)
ATTRIBUTE_NONNULL(1);
+void virStorageFileReportBrokenChain(int errcode,
+ virStorageSourcePtr src,
+ virStorageSourcePtr parent);
+
#endif /* __VIR_STORAGE_SOURCE_H__ */
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/12] security: selinux: Take parent security label into account
Until now we ignored user-provided backing chains and while detecting
the code inherited labels of the parent device. With user provided
chains we should keep this functionality, so label of the parent image
in the backing chain will be applied if an image-specific label is not
present.
---
src/security/security_selinux.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 66b3bbf1c..ed1828a12 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1597,6 +1597,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
virSecurityDeviceLabelDefPtr disk_seclabel;
+virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
int ret;
if (!src->path || !virStorageSourceIsLocalStorage(src))
@@ -1608,12 +1609,20 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_SELINUX_NAME);
+if (parent)
+parent_seclabel = virStorageSourceGetSecurityLabelDef(parent,
+
SECURITY_SELINUX_NAME);
-if (disk_seclabel && !disk_seclabel->relabel)
-return 0;
+if (disk_seclabel && (!disk_seclabel->relabel || disk_seclabel->label)) {
+if (!disk_seclabel->relabel)
+return 0;
-if (disk_seclabel && disk_seclabel->relabel && disk_seclabel->label) {
ret = virSecuritySELinuxSetFilecon(mgr, src->path,
disk_seclabel->label);
+} else if (parent_seclabel && (!parent_seclabel->relabel ||
parent_seclabel->label)) {
+if (!parent_seclabel->relabel)
+return 0;
+
+ret = virSecuritySELinuxSetFilecon(mgr, src->path,
parent_seclabel->label);
} else if (!parent || parent == src) {
if (src->shared) {
ret = virSecuritySELinuxSetFileconOptional(mgr,
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] qemu: domain: Prepare TLS data for the whole backing chain
Iterate through the backing chain when setting up TLS for disks.
---
src/qemu/qemu_domain.c | 41 ++---
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index aebe24e7b..3560cdd29 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7845,28 +7845,31 @@ int
qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
virQEMUDriverConfigPtr cfg)
{
+virStorageSourcePtr next;
-/* VxHS uses only client certificates and thus has no need for
- * the server-key.pem nor a secret that could be used to decrypt
- * the it, so no need to add a secinfo for a secret UUID. */
-if (src->type == VIR_STORAGE_TYPE_NETWORK &&
-src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) {
-
-if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT) {
-if (cfg->vxhsTLS)
-src->haveTLS = VIR_TRISTATE_BOOL_YES;
-else
-src->haveTLS = VIR_TRISTATE_BOOL_NO;
-src->tlsFromConfig = true;
-}
+for (next = src; virStorageSourceIsBacking(next); next =
next->backingStore) {
+/* VxHS uses only client certificates and thus has no need for
+ * the server-key.pem nor a secret that could be used to decrypt
+ * the it, so no need to add a secinfo for a secret UUID. */
+if (next->type == VIR_STORAGE_TYPE_NETWORK &&
+next->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) {
+
+if (next->haveTLS == VIR_TRISTATE_BOOL_ABSENT) {
+if (cfg->vxhsTLS)
+next->haveTLS = VIR_TRISTATE_BOOL_YES;
+else
+next->haveTLS = VIR_TRISTATE_BOOL_NO;
+next->tlsFromConfig = true;
+}
-if (src->haveTLS == VIR_TRISTATE_BOOL_YES) {
-/* Grab the vxhsTLSx509certdir and set the verify/listen values.
- * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */
-if (VIR_STRDUP(src->tlsCertdir, cfg->vxhsTLSx509certdir) < 0)
-return -1;
+if (next->haveTLS == VIR_TRISTATE_BOOL_YES) {
+/* Grab the vxhsTLSx509certdir and set the verify/listen
values.
+ * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */
+if (VIR_STRDUP(next->tlsCertdir, cfg->vxhsTLSx509certdir) < 0)
+return -1;
-src->tlsVerify = true;
+next->tlsVerify = true;
+}
}
}
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] storage: Add feature check for storage file backend supporting access check
When the user provides backing chain, we don't need the full support for
traversing the backing chain. This patch adds a feature check for the
virStorageSourceAccess API.
---
src/storage/storage_source.c | 20
src/storage/storage_source.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index e3c5c3285..cced5308c 100644
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -95,6 +95,26 @@ virStorageFileSupportsSecurityDriver(const virStorageSource
*src)
}
+/**
+ * virStorageFileSupportsAccess:
+ *
+ * @src: a storage file structure
+ *
+ * Check if a storage file supports checking if the storage source is
accessible
+ * for the given vm.
+ */
+bool
+virStorageFileSupportsAccess(const virStorageSource *src)
+{
+virStorageFileBackendPtr backend;
+
+if (!(backend = virStorageFileGetBackendForSupportCheck(src)))
+return false;
+
+return !!backend->storageFileAccess;
+}
+
+
void
virStorageFileDeinit(virStorageSourcePtr src)
{
diff --git a/src/storage/storage_source.h b/src/storage/storage_source.h
index 6462baf6a..320ea3cab 100644
--- a/src/storage/storage_source.h
+++ b/src/storage/storage_source.h
@@ -41,6 +41,7 @@ int virStorageFileAccess(virStorageSourcePtr src, int mode);
int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid);
bool virStorageFileSupportsSecurityDriver(const virStorageSource *src);
+bool virStorageFileSupportsAccess(const virStorageSource *src);
int virStorageFileGetMetadata(virStorageSourcePtr src,
uid_t uid, gid_t gid,
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] security: selinux: Pass parent storage source into image labeling helper
virSecuritySELinuxSetImageLabelInternal assigns different labels to
backing chain members than to the parent image. This was done via the
'first' flag. Convert it to passing in pointer to the parent
virStorageSource. This will allow us to use the parent virStorageSource
in further changes.
---
src/security/security_selinux.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index cd3e41193..66b3bbf1c 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1592,7 +1592,7 @@ static int
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src,
-bool first)
+virStorageSourcePtr parent)
{
virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
@@ -1614,7 +1614,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
if (disk_seclabel && disk_seclabel->relabel && disk_seclabel->label) {
ret = virSecuritySELinuxSetFilecon(mgr, src->path,
disk_seclabel->label);
-} else if (first) {
+} else if (!parent || parent == src) {
if (src->shared) {
ret = virSecuritySELinuxSetFileconOptional(mgr,
src->path,
@@ -1660,7 +1660,7 @@ virSecuritySELinuxSetImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
virStorageSourcePtr src)
{
-return virSecuritySELinuxSetImageLabelInternal(mgr, def, src, true);
+return virSecuritySELinuxSetImageLabelInternal(mgr, def, src, NULL);
}
@@ -1670,14 +1670,11 @@ virSecuritySELinuxSetDiskLabel(virSecurityManagerPtr
mgr,
virDomainDiskDefPtr disk)
{
-bool first = true;
virStorageSourcePtr next;
for (next = disk->src; virStorageSourceIsBacking(next); next =
next->backingStore) {
-if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, first) < 0)
+if (virSecuritySELinuxSetImageLabelInternal(mgr, def, next, disk->src)
< 0)
return -1;
-
-first = false;
}
return 0;
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] storage: Extract common code to retrieve driver backend for support check
The 'file access' module of the storage driver has few feature checks to
determine whether libvirt supports given storage driver method. The code
to retrieve the driver struct needed for the check is the same so it can
be extracted.
---
src/storage/storage_source.c | 43 +++
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index 419fa3d43..e3c5c3285 100644
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -44,24 +44,30 @@ virStorageFileIsInitialized(const virStorageSource *src)
}
-static bool
-virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
+static virStorageFileBackendPtr
+virStorageFileGetBackendForSupportCheck(const virStorageSource *src)
{
int actualType;
-virStorageFileBackendPtr backend;
if (!src)
-return false;
+return NULL;
+
+if (src->drv)
+return src->drv->backend;
+
actualType = virStorageSourceGetActualType(src);
-if (src->drv) {
-backend = src->drv->backend;
-} else {
-if (!(backend = virStorageFileBackendForTypeInternal(actualType,
- src->protocol,
- false)))
-return false;
-}
+return virStorageFileBackendForTypeInternal(actualType, src->protocol,
false);
+}
+
+
+static bool
+virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
+{
+virStorageFileBackendPtr backend;
+
+if (!(backend = virStorageFileGetBackendForSupportCheck(src)))
+return false;
return backend->storageFileGetUniqueIdentifier &&
backend->storageFileRead &&
@@ -80,21 +86,10 @@
virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
bool
virStorageFileSupportsSecurityDriver(const virStorageSource *src)
{
-int actualType;
virStorageFileBackendPtr backend;
-if (!src)
+if (!(backend = virStorageFileGetBackendForSupportCheck(src)))
return false;
-actualType = virStorageSourceGetActualType(src);
-
-if (src->drv) {
-backend = src->drv->backend;
-} else {
-if (!(backend = virStorageFileBackendForTypeInternal(actualType,
- src->protocol,
- false)))
-return false;
-}
return !!backend->storageFileChown;
}
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] qemu: domain: Destroy secrets for complete backing chain
---
src/qemu/qemu_domain.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4a2ba1761..c689911c4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1324,6 +1324,19 @@ qemuDomainSecretInfoTLSNew(virConnectPtr conn,
}
+static void
+qemuDomainSecretStorageSourceDestroy(virStorageSourcePtr src)
+{
+qemuDomainStorageSourcePrivatePtr srcPriv =
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
+
+if (srcPriv && srcPriv->secinfo)
+qemuDomainSecretInfoFree(&srcPriv->secinfo);
+
+if (srcPriv && srcPriv->encinfo)
+qemuDomainSecretInfoFree(&srcPriv->encinfo);
+}
+
+
/* qemuDomainSecretDiskDestroy:
* @disk: Pointer to a disk definition
*
@@ -1332,13 +1345,10 @@ qemuDomainSecretInfoTLSNew(virConnectPtr conn,
void
qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
{
-qemuDomainStorageSourcePrivatePtr srcPriv =
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
-
-if (srcPriv && srcPriv->secinfo)
-qemuDomainSecretInfoFree(&srcPriv->secinfo);
+virStorageSourcePtr next;
-if (srcPriv && srcPriv->encinfo)
-qemuDomainSecretInfoFree(&srcPriv->encinfo);
+for (next = disk->src; virStorageSourceIsBacking(next); next =
next->backingStore)
+qemuDomainSecretStorageSourceDestroy(next);
}
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/12] qemu: block: Prepare for user-specified backing chains (blockdev-add saga)
When users will specify backing chain, we need to take into account what was passed in them and/or inherit the data from the parents as we are copying the data (labels, etc...) from the parent disk source. Peter Krempa (12): storage: Extract common code to retrieve driver backend for support check storage: Add feature check for storage file backend supporting access check storage: Extract error reporting for broken chains security: selinux: Pass parent storage source into image labeling helper security: dac: Take parent security label into account security: selinux: Take parent security label into account qemu: domain: Simplify using DAC permissions of top of backing chain qemu: domain: Extract setup for disk source secrets qemu: domain: Destroy secrets for complete backing chain qemu: domain: Remove pointless alias check qemu: domain: Prepare TLS data for the whole backing chain qemu: domain: skip chain detection to end of backing chain src/qemu/qemu_domain.c | 177 ++-- src/qemu/qemu_domain.h | 6 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- src/security/security_dac.c | 38 +++-- src/security/security_selinux.c | 26 +++--- src/storage/storage_source.c| 110 - src/storage/storage_source.h| 5 ++ 9 files changed, 246 insertions(+), 126 deletions(-) -- 2.14.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 20.10.2017 15:36, Christian Borntraeger wrote: > > > On 10/20/2017 03:16 PM, David Hildenbrand wrote: >> >>> Hi all, >>> >>> we recently encountered the problem that the 'host-model' [1] has to be >>> related to the machine type of a domain. We have following problem: >>> >>>Let's assume we've a z13 system with a QEMU 2.9 and we define a >>>domain using the default s390-virtio-ccw machine together with the >>>host-model CPU mode [1]. The definition will have the machine >>>expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >>>in the domain definition. In a next step we upgrade to QEMU 2.10 >>>(first version to recognize z14). Everything is still fine, even >>>though the machine runs in 2.9 compatibility mode. Finally we >>>upgrade to a z14. As a consequence it is not possible to start the >>>domain anymore as the machine type doesn't support our CPU host >>>model (which is expanded at start time of the domain). >> >> Actually, what is the cause of that problem? I assume it is the gs >> feature (gs_allowed)? >> >> We should really avoid such things (..._allowed) for CPU model features >> in the future and clue all new such stuff to cpumodel_allowed. > > Yes, starting a guest with > > hvm > > > > results in > > qemu-system-s390x: Some features requested in the CPU model are not available > in the configuration: gs > > Tying it to cpumodel_allowed would not help, migration-wise though. > libvirt would still transform > > > hvm > > My point was, that the host model would have to be copied and _remain_ there when s390-ccw-virtio was expanded to s390-ccw-virtio-2.9. So really replacing by the model z13-base This would at least fix this issue. Just like s390-ccw-virtio get's replaced and remains thats way. But this might for sure have other problems. > > > into > -cpu > z14-base,aen=on,cmmnt=on,aefsi=on,mepoch=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on, > \ > msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,edat2=on,vx=on,ipter=on,vxeh=on,vxpd=on, > \ > esop=on,iep=on,cte=on,ais=on,gs=on,zpci=on,sea_esop2=on,te=on,cmm=on > ^ > because cpu model is certainly there. Now the guest would start but migration > would > later fail because what we create now would never have been possible with 2.9. Migration is a totally different story, as tooling has to make sure to find a CPU model that is compatible over all host models. So cpumodel_allowed would indeed work. (at least in my theory ;) ) > > If libvirt could get information from QEMU depending on the machine version, > this would > make it work. But of course this has other issues. I am not sure if that is the right thing to do. The documentation states clearly that the host model is copied. If that is not runnable, fix the setup. > > Christian > -- Thanks, David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 10/20/2017 03:16 PM, David Hildenbrand wrote: > >> Hi all, >> >> we recently encountered the problem that the 'host-model' [1] has to be >> related to the machine type of a domain. We have following problem: >> >>Let's assume we've a z13 system with a QEMU 2.9 and we define a >>domain using the default s390-virtio-ccw machine together with the >>host-model CPU mode [1]. The definition will have the machine >>expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >>in the domain definition. In a next step we upgrade to QEMU 2.10 >>(first version to recognize z14). Everything is still fine, even >>though the machine runs in 2.9 compatibility mode. Finally we >>upgrade to a z14. As a consequence it is not possible to start the >>domain anymore as the machine type doesn't support our CPU host >>model (which is expanded at start time of the domain). > > Actually, what is the cause of that problem? I assume it is the gs > feature (gs_allowed)? > > We should really avoid such things (..._allowed) for CPU model features > in the future and clue all new such stuff to cpumodel_allowed. Yes, starting a guest with hvm results in qemu-system-s390x: Some features requested in the CPU model are not available in the configuration: gs Tying it to cpumodel_allowed would not help, migration-wise though. libvirt would still transform hvm into -cpu z14-base,aen=on,cmmnt=on,aefsi=on,mepoch=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on, \ msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,edat2=on,vx=on,ipter=on,vxeh=on,vxpd=on, \ esop=on,iep=on,cte=on,ais=on,gs=on,zpci=on,sea_esop2=on,te=on,cmm=on ^ because cpu model is certainly there. Now the guest would start but migration would later fail because what we create now would never have been possible with 2.9. If libvirt could get information from QEMU depending on the machine version, this would make it work. But of course this has other issues. Christian -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Missing 'removeTimeoutImpl' check variable inside virEventRegisterImpl() function.
On 10/19/2017 11:35 AM, Julio Faracco wrote:
> The function virEventRegisterImpl() checks the attempt to replace the
> registered events. But there is a duplicate variable inside the IF statement.
> The variable 'removeHandleImpl' was wrongly repeated. One of them needs to be
> replaced by 'removeTimeoutImpl'.
>
> Signed-off-by: Julio Faracco
> ---
> src/util/virevent.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Introduced by commit id '5f5c515bb'
Reviewed-by: John Ferlan
will push shortly...
Tks,
John
> diff --git a/src/util/virevent.c b/src/util/virevent.c
> index 51d8714..87069e3 100644
> --- a/src/util/virevent.c
> +++ b/src/util/virevent.c
> @@ -241,7 +241,7 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle,
>addTimeout, updateTimeout, removeTimeout);
>
> if (addHandleImpl || updateHandleImpl || removeHandleImpl ||
> -addTimeoutImpl || updateTimeoutImpl || removeHandleImpl) {
> +addTimeoutImpl || updateTimeoutImpl || removeTimeoutImpl) {
> VIR_WARN("Ignoring attempt to replace registered event loop");
> return;
> }
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
> Hi all, > > we recently encountered the problem that the 'host-model' [1] has to be > related to the machine type of a domain. We have following problem: > >Let's assume we've a z13 system with a QEMU 2.9 and we define a >domain using the default s390-virtio-ccw machine together with the >host-model CPU mode [1]. The definition will have the machine >expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >in the domain definition. In a next step we upgrade to QEMU 2.10 >(first version to recognize z14). Everything is still fine, even >though the machine runs in 2.9 compatibility mode. Finally we >upgrade to a z14. As a consequence it is not possible to start the >domain anymore as the machine type doesn't support our CPU host >model (which is expanded at start time of the domain). Actually, what is the cause of that problem? I assume it is the gs feature (gs_allowed)? We should really avoid such things (..._allowed) for CPU model features in the future and clue all new such stuff to cpumodel_allowed. -- Thanks, David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Missing 'removeTimeoutImpl' check variable inside virEventRegisterImpl() function.
On Thu, 2017-10-19 at 13:35 -0200, Julio Faracco wrote: > The function virEventRegisterImpl() checks the attempt to replace the > registered events. But there is a duplicate variable inside the IF statement. > The variable 'removeHandleImpl' was wrongly repeated. One of them needs to be > replaced by 'removeTimeoutImpl'. > > Signed-off-by: Julio Faracco Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/7] Preparation for new QEMU migration states
On 10/19/2017 09:56 AM, Jiri Denemark wrote: > Mostly refactoring of the horrible mess in qemuMigrationRun. > > Jiri Denemark (7): > qemu: Use switch in qemuMigrationCompleted > qemu: Refactor qemuMigrationRun a bit > qemu: Split cleanup and error code in qemuMigrationRun > qemu: Unite error handling in qemuMigrationRun > qemu: Don't misuse "ret" in qemuMigrationRun > qemu: Consistently use exit_monitor in qemuMigrationRun > qemu: Set correct job status when qemuMigrationRun fails > > src/qemu/qemu_migration.c | 196 > +- > 1 file changed, 105 insertions(+), 91 deletions(-) > Reviewed-by: John Ferlan (series) John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 20.10.2017 14:50, Jiri Denemark wrote: > On Fri, Oct 20, 2017 at 13:37:42 +0200, David Hildenbrand wrote: >> On 20.10.2017 13:09, Marc Hartmayer wrote: >>> we recently encountered the problem that the 'host-model' [1] has to be >>> related to the machine type of a domain. We have following problem: >>> >>>Let's assume we've a z13 system with a QEMU 2.9 and we define a >>>domain using the default s390-virtio-ccw machine together with the >>>host-model CPU mode [1]. The definition will have the machine >>>expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >>>in the domain definition. In a next step we upgrade to QEMU 2.10 >>>(first version to recognize z14). Everything is still fine, even >>>though the machine runs in 2.9 compatibility mode. Finally we >>>upgrade to a z14. As a consequence it is not possible to start the >>>domain anymore as the machine type doesn't support our CPU host >>>model (which is expanded at start time of the domain). >> >> Yes, the host model may vary depending on QEMU version and to some >> degree also on compatibility machines (although I always try to push >> people to not introduce any new such stuff). >> >> Quoting for the libvirt documentation: https://libvirt.org/formatdomain.html >> >> "host-model >> The host-model mode is essentially a shortcut to copying host CPU >> definition from capabilities XML into domain XML. Since the CPU >> definition is copied just before starting a domain, exactly the same XML >> can be used on different hosts while still providing the best guest CPU >> each host supports." >> >> You're telling me that that copying does not happen, which seems to be >> the problematic part about this in my opinion. >> >> So I am not sure if probing anything else (as you noted below) is the >> correct thing to do. Copy it and you have a migration_safe and even >> static version of the _current_ host CPU. > > The thing is libvirt calls query-cpu-model-expansion to check what the > host CPU is. This 'host-model' CPU is replaced with the probed CPU model > when a domain starts. The problem described by Marc is that the probed > CPU model cannot be used universally with all machine types. So starting > a domain on such host with machine type s390-virtio-ccw-2.10 works, but > a domain with machine type s390-virtio-ccw-2.9 fails to start with the > same probed CPU model. > My assumption would be that the CPU model is copied into the XML when the domain fist starts. This is what the documentation describes. So when upgrading QEMU, the CPU model in the XML is still the same (z13), even though something different is now reported in the host info after upgrading QEMU (z14). In this case it would continue to work. The problem is that the CPU model is not copied into the XML doesn't remain there, right? It is suddenly replaced with a z14 host model. > Jirka > -- Thanks, David -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/2] guests: Minor fixes and tweaks
On Fri, Oct 20, 2017 at 01:31:17PM +0200, Andrea Bolognani wrote: > 1/2 is a bug fix, 2/2 a small improvement. > > Andrea Bolognani (2): > guests: Reorder configuration steps for root authentication > guests: Don't warn when bootstrapping package manager Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: Remove unnecessary virtio disk detach info.alias check
On 10/20/2017 08:21 AM, Ján Tomko wrote:
> On Fri, Oct 20, 2017 at 08:03:29AM -0400, John Ferlan wrote:
>> Since qemuAssignDeviceDiskAlias checks whether the input info.alias
>> is already present, no need to check here as well.
>>
>> Signed-off-by: John Ferlan
>> ---
>> src/qemu/qemu_hotplug.c | 6 ++
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 51a7a68f84..9fbb3a0712 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -4447,10 +4447,8 @@
>> qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
>> goto cleanup;
>> }
>>
>> - if (!detach->info.alias) {
>> - if (qemuAssignDeviceDiskAlias(vm->def, detach,
>> priv->qemuCaps) < 0)
>> - goto cleanup;
>> - }
>> + if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
>> + goto cleanup;
>>
>
> All the calls assigning aliases in the Detach functions are
> unnecessary. At this point, all the domain's devices should have their
> aliases assigned. If by any case they do not, it is an error in other
> part of the libvirt code.
True - I was following the symptom though... That being calling
qemuMonitorDelDevice with a NULL detach->info.alias means
qemuMonitorTextDelDevice could dereference @devalias in the call to
qemuMonitorEscapeArg. In the json path, failure will come during
qemuMonitorJSONMakeCommand because "s:id" requires a NON NULL value in
virJSONValueObjectAddVArgs.
Anyway this just seemed to be the "easiest" adjustment especially since
no other caller checks if !*->info.alias before calling
qemuAssignDeviceDiskAlias (similar for Controller too) because the top
of each checks if already assigned, return 0.
>
> I was going to send patches cleaning these up, but I could not decide
> whether to report an error if we find a device without an alias,
> or to just quietly assume that an alias. And I did not want to conflict
> with Michal's series again.
>
> Jan
>
Still I do believe it indicates that providing an error message is
probably better than blindly hoping things will work. I wasn't following
the user alias series that closely so I wasn't thinking about that.
I'm perfectly fine with just dropping this and the next patch since at
this point it's just Coverity noise that I can easily filter out until
something better is provided.
John
>> qemuDomainMarkDeviceForRemoval(vm, &detach->info);
>>
>> --
>> 2.13.6
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On Fri, Oct 20, 2017 at 13:37:42 +0200, David Hildenbrand wrote: > On 20.10.2017 13:09, Marc Hartmayer wrote: > > we recently encountered the problem that the 'host-model' [1] has to be > > related to the machine type of a domain. We have following problem: > > > >Let's assume we've a z13 system with a QEMU 2.9 and we define a > >domain using the default s390-virtio-ccw machine together with the > >host-model CPU mode [1]. The definition will have the machine > >expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode > >in the domain definition. In a next step we upgrade to QEMU 2.10 > >(first version to recognize z14). Everything is still fine, even > >though the machine runs in 2.9 compatibility mode. Finally we > >upgrade to a z14. As a consequence it is not possible to start the > >domain anymore as the machine type doesn't support our CPU host > >model (which is expanded at start time of the domain). > > Yes, the host model may vary depending on QEMU version and to some > degree also on compatibility machines (although I always try to push > people to not introduce any new such stuff). > > Quoting for the libvirt documentation: https://libvirt.org/formatdomain.html > > "host-model > The host-model mode is essentially a shortcut to copying host CPU > definition from capabilities XML into domain XML. Since the CPU > definition is copied just before starting a domain, exactly the same XML > can be used on different hosts while still providing the best guest CPU > each host supports." > > You're telling me that that copying does not happen, which seems to be > the problematic part about this in my opinion. > > So I am not sure if probing anything else (as you noted below) is the > correct thing to do. Copy it and you have a migration_safe and even > static version of the _current_ host CPU. The thing is libvirt calls query-cpu-model-expansion to check what the host CPU is. This 'host-model' CPU is replaced with the probed CPU model when a domain starts. The problem described by Marc is that the probed CPU model cannot be used universally with all machine types. So starting a domain on such host with machine type s390-virtio-ccw-2.10 works, but a domain with machine type s390-virtio-ccw-2.9 fails to start with the same probed CPU model. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On Fri, Oct 20, 2017 at 13:09:26 +0200, Marc Hartmayer wrote: > we recently encountered the problem that the 'host-model' [1] has to be > related to the machine type of a domain. We have following problem: > >Let's assume we've a z13 system with a QEMU 2.9 and we define a >domain using the default s390-virtio-ccw machine together with the >host-model CPU mode [1]. The definition will have the machine >expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >in the domain definition. In a next step we upgrade to QEMU 2.10 >(first version to recognize z14). Everything is still fine, even >though the machine runs in 2.9 compatibility mode. Finally we >upgrade to a z14. As a consequence it is not possible to start the >domain anymore as the machine type doesn't support our CPU host >model (which is expanded at start time of the domain). > > For determining the actual host-model the QMP command > 'query-cpu-model-expansion' is used. This is done once per QEMU binary > and the result of it is cached by libvirt. The problem with that is > that libvirt queries with the newest machine type of the QEMU binary > for the host CPU model. No, libvirt probes QEMU with -machine none. > We could now either probe the host CPU model for each QEMU binary + > machine type combination and for this we've to start a new QEMU > process each time. This is not really a viable solution. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [go PATCH 0/2] Add missing bindings
On Fri, Oct 20, 2017 at 02:27:44PM +0200, Pavel Hrdina wrote: > Pavel Hrdina (2): > Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant > Add virDomainSetLifecycleAction API support > > domain.go| 39 +++ > domain_compat.go | 12 > domain_compat.h | 47 +++ > 3 files changed, 98 insertions(+) Reviewed-by: Daniel P. Berrange 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] [go PATCH 0/2] Add missing bindings
Pavel Hrdina (2): Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant Add virDomainSetLifecycleAction API support domain.go| 39 +++ domain_compat.go | 12 domain_compat.h | 47 +++ 3 files changed, 98 insertions(+) -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [go PATCH 1/2] Add VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE constant
Signed-off-by: Pavel Hrdina
---
domain.go | 6 ++
domain_compat.h | 6 ++
2 files changed, 12 insertions(+)
diff --git a/domain.go b/domain.go
index b83bfc3..5183726 100644
--- a/domain.go
+++ b/domain.go
@@ -2860,6 +2860,8 @@ type DomainJobInfo struct {
MemBpsuint64
MemDirtyRateSet bool
MemDirtyRate uint64
+ MemPageSizeSetbool
+ MemPageSize uint64
MemIterationSet bool
MemIteration uint64
DiskTotalSet bool
@@ -2992,6 +2994,10 @@ func getDomainJobInfoFieldInfo(params *DomainJobInfo)
map[string]typedParamsFiel
set: ¶ms.MemDirtyRateSet,
ul: ¶ms.MemDirtyRate,
},
+ C.VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE: typedParamsFieldInfo{
+ set: ¶ms.MemPageSizeSet,
+ ul: ¶ms.MemPageSize,
+ },
C.VIR_DOMAIN_JOB_MEMORY_ITERATION: typedParamsFieldInfo{
set: ¶ms.MemIterationSet,
ul: ¶ms.MemIteration,
diff --git a/domain_compat.h b/domain_compat.h
index 0f66e8b..afef84d 100644
--- a/domain_compat.h
+++ b/domain_compat.h
@@ -971,4 +971,10 @@ int virDomainManagedSaveDefineXMLCompat(virDomainPtr
domain,
const char *dxml,
unsigned int flags);
+/* 3.9.0 */
+
+#ifndef VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE
+#define VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE "memory_page_size"
+#endif
+
#endif /* LIBVIRT_GO_DOMAIN_COMPAT_H__ */
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [go PATCH 2/2] Add virDomainSetLifecycleAction API support
Signed-off-by: Pavel Hrdina
---
domain.go| 33 +
domain_compat.go | 12
domain_compat.h | 41 +
3 files changed, 86 insertions(+)
diff --git a/domain.go b/domain.go
index 5183726..36d77ba 100644
--- a/domain.go
+++ b/domain.go
@@ -4504,3 +4504,36 @@ func (d *Domain) ManagedSaveGetXMLDesc(flags uint32)
(string, error) {
C.free(unsafe.Pointer(ret))
return xml, nil
}
+
+type DomainLifecycle int
+
+const (
+ DOMAIN_LIFECYCLE_POWEROFF =
DomainLifecycle(C.VIR_DOMAIN_LIFECYCLE_POWEROFF)
+ DOMAIN_LIFECYCLE_REBOOT =
DomainLifecycle(C.VIR_DOMAIN_LIFECYCLE_REBOOT)
+ DOMAIN_LIFECYCLE_CRASH=
DomainLifecycle(C.VIR_DOMAIN_LIFECYCLE_CRASH)
+)
+
+type DomainLifecycleAction int
+
+const (
+ DOMAIN_LIFECYCLE_ACTION_DESTROY =
DomainLifecycleAction(C.VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY)
+ DOMAIN_LIFECYCLE_ACTION_RESTART =
DomainLifecycleAction(C.VIR_DOMAIN_LIFECYCLE_ACTION_RESTART)
+ DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME =
DomainLifecycleAction(C.VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME)
+ DOMAIN_LIFECYCLE_ACTION_PRESERVE =
DomainLifecycleAction(C.VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE)
+ DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY =
DomainLifecycleAction(C.VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY)
+ DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART =
DomainLifecycleAction(C.VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART)
+)
+
+// See also
https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetLifecycleAction
+func (d *Domain) SetLifecycleAction(lifecycleType uint32, action uint32, flags
uint32) error {
+ if C.LIBVIR_VERSION_NUMBER < 3009000 {
+ return GetNotImplementedError("virDomainSetLifecycleAction")
+ }
+
+ ret := C.virDomainSetLifecycleActionCompat(d.ptr,
C.uint(lifecycleType), C.uint(action), C.uint(flags))
+ if ret == -1 {
+ return GetLastError()
+ }
+
+ return nil
+}
diff --git a/domain_compat.go b/domain_compat.go
index c59b00e..eada95c 100644
--- a/domain_compat.go
+++ b/domain_compat.go
@@ -345,5 +345,17 @@ int virDomainManagedSaveDefineXMLCompat(virDomainPtr
domain,
#endif
}
+int virDomainSetLifecycleActionCompat(virDomainPtr domain,
+ unsigned int type,
+ unsigned int action,
+ unsigned int flags)
+{
+#if LIBVIR_VERSION_NUMBER < 3009000
+assert(0); // Caller should have checked version
+#else
+return virDomainSetLifecycleAction(domain, type, action, flags);
+#endif
+}
+
*/
import "C"
diff --git a/domain_compat.h b/domain_compat.h
index afef84d..2793b1b 100644
--- a/domain_compat.h
+++ b/domain_compat.h
@@ -977,4 +977,45 @@ int virDomainManagedSaveDefineXMLCompat(virDomainPtr
domain,
#define VIR_DOMAIN_JOB_MEMORY_PAGE_SIZE "memory_page_size"
#endif
+#ifndef VIR_DOMAIN_LIFECYCLE_POWEROFF
+#define VIR_DOMAIN_LIFECYCLE_POWEROFF 0
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_REBOOT
+#define VIR_DOMAIN_LIFECYCLE_REBOOT 1
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_CRASH
+#define VIR_DOMAIN_LIFECYCLE_CRASH 2
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY
+#define VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY 0
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_ACTION_RESTART
+#define VIR_DOMAIN_LIFECYCLE_ACTION_RESTART 1
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME
+#define VIR_DOMAIN_LIFECYCLE_ACTION_RESTART_RENAME 2
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE
+#define VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE 3
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY
+#define VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_DESTROY 4
+#endif
+
+#ifndef VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART
+#define VIR_DOMAIN_LIFECYCLE_ACTION_COREDUMP_RESTART 5
+#endif
+
+int virDomainSetLifecycleActionCompat(virDomainPtr domain,
+ unsigned int type,
+ unsigned int action,
+ unsigned int flags);
+
#endif /* LIBVIRT_GO_DOMAIN_COMPAT_H__ */
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: Remove unnecessary virtio disk detach info.alias check
On Fri, Oct 20, 2017 at 08:03:29AM -0400, John Ferlan wrote:
Since qemuAssignDeviceDiskAlias checks whether the input info.alias
is already present, no need to check here as well.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 51a7a68f84..9fbb3a0712 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4447,10 +4447,8 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
goto cleanup;
}
-if (!detach->info.alias) {
-if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
-goto cleanup;
-}
+if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
+goto cleanup;
All the calls assigning aliases in the Detach functions are
unnecessary. At this point, all the domain's devices should have their
aliases assigned. If by any case they do not, it is an error in other
part of the libvirt code.
I was going to send patches cleaning these up, but I could not decide
whether to report an error if we find a device without an alias,
or to just quietly assume that an alias. And I did not want to conflict
with Michal's series again.
Jan
qemuDomainMarkDeviceForRemoval(vm, &detach->info);
--
2.13.6
--
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] [RFC] Question regarding the validity of QEMU capabilities
On Fri, Oct 20, 2017 at 01:10:14PM +0200, Pavel Hrdina wrote: > On Fri, Oct 20, 2017 at 12:59:51PM +0200, Marc Hartmayer wrote: > > Hi all, > > > > the actual capabilities of QEMU are depending on the host. This > > includes dependencies like which kernel modules are loaded or which > > kernel parameters are used (e.g. kvm.nested). Therefore, after a > > restart we cannot be sure that the QEMU capabilities remain the same. > > > > How can we solve this problem? > > Hi, > > thanks for bringing up this issue, we kind of already know about it > but it's good idea to discuss it publicly. > > > > > I have come up with two ways: > > - reprobe the capabilities with every host reboot > > This is the solution that was agreed on but nobody was motivated enough > to write the code :). Reprobing QEMU is quite heavy. I wonder if we should change slightly. Make our capabilities cache *only* contain stuff reported by the QEMU binary. For other stuff we detect from the host, never cache it. That way we can just revalidate the host stuff on every libvirtd startup, but keep the slow/heavy QEMU cache untouched > > > - check for every possible change in virQEMUCapsIsValid... (this is > >already done for KVM). In my opinion this is not the way to go. 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 0/3] qemu: Fix race condition between block jobs and the end of migration
When migrating both memory and storage, we need to make sure block jobs are finished after the virtual CPUs get paused at the end of migration but before QEMU completes the migration otherwise QEMU may die with _co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed This is a libvirt side of the QEMU fix from Dr. David Alan Gilbert: http://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg04895.html Applicable on top of my "Preparation for new QEMU migration states" series I sent yesterday. Jiri Denemark (3): qemu: Add support for migrate-continue QMP command qemu: Add pause-before-switchover migration capability qemu: Enabled pause-before-switchover migration capability src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_migration.c| 75 +++- src/qemu/qemu_monitor.c | 18 +-- src/qemu/qemu_monitor.h | 6 src/qemu/qemu_monitor_json.c | 29 + src/qemu/qemu_monitor_json.h | 4 +++ 7 files changed, 131 insertions(+), 3 deletions(-) -- 2.14.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: Add pause-before-switchover migration capability
This new capability enables a pause before device state serialization so
that we can finish all block jobs without racing with the end of the
migration. The pause is indicated by "pre-switchover" state. Once we're
done QEMU enters "device" migration state.
This patch just defines the new capability and QEMU migration states and
their mapping to our job states.
Signed-off-by: Jiri Denemark
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_migration.c| 11 +++
src/qemu/qemu_monitor.c | 5 +++--
src/qemu/qemu_monitor.h | 3 +++
src/qemu/qemu_monitor_json.c | 2 ++
6 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ece8ee7dd..abf65094a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -423,6 +423,7 @@ qemuDomainJobStatusToType(qemuDomainJobStatus status)
case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
+case QEMU_DOMAIN_JOB_STATUS_PAUSED:
return VIR_DOMAIN_JOB_UNBOUNDED;
case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 3b4272047..ff5328277 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -103,6 +103,7 @@ typedef enum {
QEMU_DOMAIN_JOB_STATUS_ACTIVE,
QEMU_DOMAIN_JOB_STATUS_MIGRATING,
QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED,
+QEMU_DOMAIN_JOB_STATUS_PAUSED,
QEMU_DOMAIN_JOB_STATUS_POSTCOPY,
QEMU_DOMAIN_JOB_STATUS_COMPLETED,
QEMU_DOMAIN_JOB_STATUS_FAILED,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 626b4e3ee..4b356002f 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1366,6 +1366,14 @@ qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_CANCELED;
break;
+case QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER:
+jobInfo->status = QEMU_DOMAIN_JOB_STATUS_PAUSED;
+break;
+
+case QEMU_MONITOR_MIGRATION_STATUS_DEVICE:
+jobInfo->status = QEMU_DOMAIN_JOB_STATUS_MIGRATING;
+break;
+
case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE:
case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING:
@@ -1459,6 +1467,7 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
+case QEMU_DOMAIN_JOB_STATUS_PAUSED:
break;
}
@@ -1474,6 +1483,7 @@ enum qemuMigrationCompletedFlags {
QEMU_MIGRATION_COMPLETED_ABORT_ON_ERROR = (1 << 0),
QEMU_MIGRATION_COMPLETED_CHECK_STORAGE = (1 << 1),
QEMU_MIGRATION_COMPLETED_POSTCOPY = (1 << 2),
+QEMU_MIGRATION_COMPLETED_PRE_SWITCHOVER = (1 << 3),
};
@@ -1534,6 +1544,7 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
switch (jobInfo->status) {
case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
+case QEMU_DOMAIN_JOB_STATUS_PAUSED:
/* The migration was aborted by us rather than QEMU itself. */
jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
return -2;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5ca3cdce2..dd9d64a20 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -172,14 +172,15 @@ VIR_ONCE_GLOBAL_INIT(qemuMonitor)
VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
QEMU_MONITOR_MIGRATION_STATUS_LAST,
"inactive", "setup",
- "active", "postcopy-active",
+ "active", "pre-switchover",
+ "device", "postcopy-active",
"completed", "failed",
"cancelling", "cancelled")
VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
QEMU_MONITOR_MIGRATION_CAPS_LAST,
"xbzrle", "auto-converge", "rdma-pin-all", "events",
- "postcopy-ram", "compress")
+ "postcopy-ram", "compress", "pause-before-switchover")
VIR_ENUM_IMPL(qemuMonitorVMStatus,
QEMU_MONITOR_VM_STATUS_LAST,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index fe29f484e..bc8494fae 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -641,6 +641,8 @@ typedef enum {
QEMU_MONITOR_MIGRATION_STATUS_INACTIVE,
QEMU_MONITOR_MIGRATION_STATUS_SETUP,
QEMU_MONITOR_MIGRATION_STATUS_ACTIVE,
+QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER,
+QEMU_MONITOR_MIGRATION_STATUS_DEVICE,
QEMU_MONITOR_MIGRATION_STATUS_POSTCOPY,
QEMU_MONITOR_MIGRATION_STATUS_COMPLETED,
QEMU_MONITOR_MIGRATION_STATUS_ERROR,
@@ -706,6 +708,7 @@ typedef enum {
QEMU_MONITOR_MIGRATION_CAPS_EVENTS,
QEMU_MONITOR_MIGRATION_CAPS_POSTCOPY,
QEMU_MONITOR_MIGRATION_CAPS_COMPRESS,
+QEMU_MONITOR_MIGRATION_CAPS_PAUSE_BEFORE_SWITCHOVER,
[libvirt] [PATCH 3/3] qemu: Enabled pause-before-switchover migration capability
QEMU identified a race condition between the device state serialization
and the end of storage migration. Both QEMU and libvirt needs to be
updated to fix this.
Our migration work flow is modified so that after starting the migration
we to wait for QEMU to enter "pre-switchover", "postcopy-active", or
"completed" state. Once there, we cancel all block jobs as usual. But if
QEMU is in "pre-switchover", we need to resume the migration afterwards
and wait again for the real end (either "postcopy-active" or
"completed" state).
Old QEMU will just enter either "postcopy-active" or "completed"
directly, which is still correctly handled even by new libvirt. The
"pre-switchover" state will only be entered if QEMU supports it and the
pause-before-switchover capability was enabled. Thus all combinations of
libvirt and QEMU will work, but only new QEMU with new libvirt will
avoid the race condition.
Signed-off-by: Jiri Denemark
---
src/qemu/qemu_migration.c | 64 ++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4b356002f..af744661f 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1525,6 +1525,16 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
goto error;
}
+/* Migration was paused before serializing device state, let's return to
+ * the caller so that it can finish all block jobs, resume migration, and
+ * wait again for the real end of the migration.
+ */
+if (flags & QEMU_MIGRATION_COMPLETED_PRE_SWITCHOVER &&
+jobInfo->status == QEMU_DOMAIN_JOB_STATUS_PAUSED) {
+VIR_DEBUG("Migration paused before switchover");
+return 1;
+}
+
/* In case of postcopy the source considers migration completed at the
* moment it switched from active to postcopy-active state. The destination
* will continue waiting until the migrate state changes to completed.
@@ -3600,6 +3610,28 @@ qemuMigrationConnect(virQEMUDriverPtr driver,
return ret;
}
+
+static int
+qemuMigrationContinue(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuMonitorMigrationStatus status,
+ qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+ret = qemuMonitorMigrateContinue(priv->mon, status);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
+
+
static int
qemuMigrationRun(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -3769,6 +3801,12 @@ qemuMigrationRun(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
goto error;
+if (qemuMigrationCapsGet(vm,
QEMU_MONITOR_MIGRATION_CAPS_PAUSE_BEFORE_SWITCHOVER) &&
+qemuMigrationSetOption(driver, vm,
+
QEMU_MONITOR_MIGRATION_CAPS_PAUSE_BEFORE_SWITCHOVER,
+ true, QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
+goto error;
+
if (qemuMigrationSetParams(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
migParams) < 0)
goto error;
@@ -3847,7 +3885,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
fd = -1;
}
-waitFlags = 0;
+waitFlags = QEMU_MIGRATION_COMPLETED_PRE_SWITCHOVER;
if (abort_on_error)
waitFlags |= QEMU_MIGRATION_COMPLETED_ABORT_ON_ERROR;
if (mig->nbd)
@@ -3889,6 +3927,30 @@ qemuMigrationRun(virQEMUDriverPtr driver,
dconn) < 0)
goto error;
+/* When migration was paused before serializing device state we need to
+ * resume it now once we finished all block jobs and wait for the real
+ * end of the migration.
+ */
+if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_PAUSED) {
+if (qemuMigrationContinue(driver, vm,
+ QEMU_MONITOR_MIGRATION_STATUS_PRE_SWITCHOVER,
+ QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
+goto error;
+
+waitFlags ^= QEMU_MIGRATION_COMPLETED_PRE_SWITCHOVER;
+
+rc = qemuMigrationWaitForCompletion(driver, vm,
+QEMU_ASYNC_JOB_MIGRATION_OUT,
+dconn, waitFlags);
+if (rc == -2) {
+goto error;
+} else if (rc == -1) {
+/* QEMU reported failed migration, nothing to cancel anymore */
+cancel = false;
+goto error;
+}
+}
+
if (iothread) {
qemuMigrationIOThreadPtr io;
--
2.14.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] conf, qemu: Check for NULL addrs in virDomainUSBAddressEnsure
Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0.
Signed-off-by: John Ferlan
---
src/conf/domain_addr.c | 3 +++
src/conf/domain_addr.h | 2 +-
src/qemu/qemu_hotplug.c | 23 ---
3 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index a44f964701..78ff7a9cc6 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -2154,6 +2154,9 @@ int
virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
{
+if (!addrs)
+return 0;
+
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
(info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
!virDomainUSBAddressPortIsValid(info->addr.usb.port))) {
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 7565322bd2..d3541bab09 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -325,7 +325,7 @@ virDomainUSBAddressAssign(virDomainUSBAddressSetPtr addrs,
int
virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ATTRIBUTE_NONNULL(2);
int
virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 10cdba436b..51a7a68f84 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -664,10 +664,8 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
-if (priv->usbaddrs) {
-if (virDomainUSBAddressEnsure(priv->usbaddrs, &disk->info) < 0)
-return -1;
-}
+if (virDomainUSBAddressEnsure(priv->usbaddrs, &disk->info) < 0)
+return -1;
if (qemuDomainAttachDiskGeneric(conn, driver, vm, disk) < 0) {
virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
@@ -1772,8 +1770,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm,
return -1;
return 1;
-} else if (priv->usbaddrs &&
- chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+} else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
if (virDomainUSBAddressEnsure(priv->usbaddrs, &chr->info) < 0)
return -1;
@@ -2187,10 +2184,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
bool teardowndevice = false;
int ret = -1;
-if (priv->usbaddrs) {
-if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
-return -1;
-}
+if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
+return -1;
if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, &hostdev, 1, 0) <
0)
goto cleanup;
@@ -2750,11 +2745,9 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver,
if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev, "input") < 0)
return -1;
} else if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) {
-if (priv->usbaddrs) {
-if (virDomainUSBAddressEnsure(priv->usbaddrs, &input->info) < 0)
-goto cleanup;
-releaseaddr = true;
-}
+if (virDomainUSBAddressEnsure(priv->usbaddrs, &input->info) < 0)
+goto cleanup;
+releaseaddr = true;
}
if (qemuAssignDeviceInputAlias(vm->def, input, -1) < 0)
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: Add support for migrate-continue QMP command
Signed-off-by: Jiri Denemark
---
src/qemu/qemu_monitor.c | 13 +
src/qemu/qemu_monitor.h | 3 +++
src/qemu/qemu_monitor_json.c | 27 +++
src/qemu/qemu_monitor_json.h | 4
4 files changed, 47 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 64efb89e8..5ca3cdce2 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4254,6 +4254,19 @@ qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon)
return qemuMonitorJSONMigrateStartPostCopy(mon);
}
+
+int
+qemuMonitorMigrateContinue(qemuMonitorPtr mon,
+ qemuMonitorMigrationStatus status)
+{
+VIR_DEBUG("status=%s", qemuMonitorMigrationStatusTypeToString(status));
+
+QEMU_CHECK_MONITOR_JSON(mon);
+
+return qemuMonitorJSONMigrateContinue(mon, status);
+}
+
+
int
qemuMonitorGetRTCTime(qemuMonitorPtr mon,
struct tm *tm)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 1e6b97714..fe29f484e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1119,6 +1119,9 @@ int qemuMonitorMigrateIncoming(qemuMonitorPtr mon,
int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon);
+int qemuMonitorMigrateContinue(qemuMonitorPtr mon,
+ qemuMonitorMigrationStatus status);
+
int qemuMonitorGetRTCTime(qemuMonitorPtr mon,
struct tm *tm);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f7567eb77..def80882c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7374,6 +7374,33 @@ qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon)
return ret;
}
+
+int
+qemuMonitorJSONMigrateContinue(qemuMonitorPtr mon,
+ qemuMonitorMigrationStatus status)
+{
+const char *statusStr = qemuMonitorMigrationStatusTypeToString(status);
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("migrate-continue",
+ "s:state", statusStr,
+ NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+goto cleanup;
+
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
int
qemuMonitorJSONGetRTCTime(qemuMonitorPtr mon,
struct tm *tm)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index b17348a11..739a99293 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -500,6 +500,10 @@ int qemuMonitorJSONMigrateIncoming(qemuMonitorPtr mon,
int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon)
ATTRIBUTE_NONNULL(1);
+int qemuMonitorJSONMigrateContinue(qemuMonitorPtr mon,
+ qemuMonitorMigrationStatus status)
+ATTRIBUTE_NONNULL(1);
+
int qemuMonitorJSONGetRTCTime(qemuMonitorPtr mon,
struct tm *tm)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
--
2.14.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] qemu: Remove unnecessary controller detach info.alias check
Since qemuAssignDeviceControllerAlias checks whether the input info.alias
is already present, no need to check here as well.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9fbb3a0712..0415584b55 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4662,10 +4662,8 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr
driver,
goto cleanup;
}
-if (!detach->info.alias) {
-if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, detach) <
0)
-goto cleanup;
-}
+if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, detach) < 0)
+goto cleanup;
qemuDomainMarkDeviceForRemoval(vm, &detach->info);
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] conf, qemu: Check for NULL addrs in virDomainUSBAddressRelease
Rather than having the caller check, if the input @addrs is NULL
(e.g. priv->usbaddrs), then just return 0.
Signed-off-by: John Ferlan
---
src/conf/domain_addr.c | 2 +-
src/conf/domain_addr.h | 2 +-
src/qemu/qemu_domain_address.c | 7 ++-
src/qemu/qemu_hotplug.c| 10 +++---
4 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 6422682391..a44f964701 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -2177,7 +2177,7 @@ virDomainUSBAddressRelease(virDomainUSBAddressSetPtr
addrs,
int targetPort;
int ret = -1;
-if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB ||
+if (!addrs || info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB ||
!virDomainUSBAddressPortIsValid(info->addr.usb.port))
return 0;
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 1731014656..7565322bd2 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -330,5 +330,5 @@ virDomainUSBAddressEnsure(virDomainUSBAddressSetPtr addrs,
int
virDomainUSBAddressRelease(virDomainUSBAddressSetPtr addrs,
virDomainDeviceInfoPtr info)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ATTRIBUTE_NONNULL(2);
#endif /* __DOMAIN_ADDR_H__ */
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 7f4ac0f45a..7d2e2e75d9 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2895,11 +2895,8 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
if (virDeviceInfoPCIAddressPresent(info))
virDomainPCIAddressReleaseAddr(priv->pciaddrs, &info->addr.pci);
-if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB &&
-priv->usbaddrs &&
-virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
-VIR_WARN("Unable to release USB address on %s",
- NULLSTR(devstr));
+if (virDomainUSBAddressRelease(priv->usbaddrs, info) < 0)
+VIR_WARN("Unable to release USB address on %s", NULLSTR(devstr));
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 91f7f9ed62..10cdba436b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2181,7 +2181,6 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
char *devstr = NULL;
-bool releaseaddr = false;
bool added = false;
bool teardowncgroup = false;
bool teardownlabel = false;
@@ -2190,8 +2189,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
if (priv->usbaddrs) {
if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
-goto cleanup;
-releaseaddr = true;
+return -1;
}
if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, &hostdev, 1, 0) <
0)
@@ -2244,8 +2242,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to remove host device from /dev");
if (added)
qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1);
-if (releaseaddr)
-virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
+virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
}
VIR_FREE(devstr);
return ret;
@@ -3684,8 +3681,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
dev.type = VIR_DOMAIN_DEVICE_DISK;
dev.data.disk = disk;
ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name));
-if (priv->usbaddrs)
-virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
+virDomainUSBAddressRelease(priv->usbaddrs, &disk->info);
virDomainDiskDefFree(disk);
return 0;
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Couple more hotplug cleanups
Patches speak for themselves - they also resolve a couple of new Coverity whines. John Ferlan (4): conf,qemu: Check for NULL addrs in virDomainUSBAddressRelease conf,qemu: Check for NULL addrs in virDomainUSBAddressEnsure qemu: Remove unnecessary virtio disk detach info.alias check qemu: Remove unnecessary controller detach info.alias check src/conf/domain_addr.c | 5 - src/conf/domain_addr.h | 4 ++-- src/qemu/qemu_domain_address.c | 7 ++- src/qemu/qemu_hotplug.c| 43 ++ 4 files changed, 22 insertions(+), 37 deletions(-) -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] qemu: Remove unnecessary virtio disk detach info.alias check
Since qemuAssignDeviceDiskAlias checks whether the input info.alias
is already present, no need to check here as well.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 51a7a68f84..9fbb3a0712 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4447,10 +4447,8 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
goto cleanup;
}
-if (!detach->info.alias) {
-if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
-goto cleanup;
-}
+if (qemuAssignDeviceDiskAlias(vm->def, detach, priv->qemuCaps) < 0)
+goto cleanup;
qemuDomainMarkDeviceForRemoval(vm, &detach->info);
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/14] Never ending story of user supplied aliases
On 10/20/2017 01:38 PM, Pavel Hrdina wrote: > On Fri, Oct 20, 2017 at 01:20:20PM +0200, Michal Privoznik wrote: >> On 10/19/2017 03:33 PM, Pavel Hrdina wrote: >>> On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote: As discussed earlier [1], we should allow users to set device aliases at the define time. While the discussed approach calls for generating missing aliases too, I'm saving that for another patch set. There are couple of reasons for that: a) I don't think it's really necessary (if users are interested in a device they can just set the alias). b) This patch set is already big enough as is. c) Generating aliases is going to have its own problems. Therefore, for now I'm only proposing parsing user supplied aliases. The idea is that it's not enabled by default for all drivers. Any driver that want to/can provide this feature has to set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some drivers that don't have notion of device aliases. But the code is generic enough so that it should be easy to use in the drivers that do know what aliases are. >>> >>> This patch series missed some of the important parts of code >>> that relies on our generated aliases: >>> >>> 1. qemuGetNextChrDevIndex() ... this will fail while attaching new >>>chardev device without an alias if there is one already provided >>>by user and doesn't match the one that we generate. >>> >>> 2. qemuAssignDeviceRedirdevAlias() ... same as 1) >>> >>> 3. qemuAssignDeviceShmemAlias() ... same as 1) >> >> Okay, these are easy to resolve. >> >>> >>> 4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging >>>network interface with user alias will fail because the alias is used >>>to figure out VLAN of the interface. >> >> This one. Well, this function is called only for ancient QEMUs (those >> which lack QMP, and even not for all of them). So do we care? Sure, I >> can cripple my code and allow user aliases only if running sufficiently >> new QEMU. Or just shrug and walk away. > > In that case just ignore it, it will print an error message and since > this will affect only hot-plug of network devices with user alias > specified we can claim that this operation is unsupported. Yeah. Just don't forget the ancient QEMU part. So the whole story is that if you're running QEMU that's lacking -netdev, and you want to hotplug an interface with user supplied alias, we error out. Yeah, I'm okay with that. We have to draw a line somewhere and say: yeah, it's probably bad behaviour, be we don't care. Patches are welcome. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
On Fri, Oct 20, 2017 at 07:08:37 -0400, John Ferlan wrote: > > > On 10/20/2017 03:03 AM, Jiri Denemark wrote: > > On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote: > >> > >> > >> On 10/18/2017 07:29 AM, Jiri Denemark wrote: > >>> Each time we need to check whether a given migration capability is > >>> supported by QEMU, we call query-migrate-capabilities QMP command and > >>> lookup the capability in the returned list. Asking for the list of > >>> supported capabilities once when we connect to QEMU and storing the > >>> result in a bitmap is much better and we don't need to enter a monitor > >>> just to check whether a migration capability is supported. > >>> > >>> Signed-off-by: Jiri Denemark > >>> --- > >>> src/qemu/qemu_domain.c | 68 > >>> + > >>> src/qemu/qemu_domain.h | 9 +++ > >>> src/qemu/qemu_process.c | 13 +- > >>> 3 files changed, 78 insertions(+), 12 deletions(-) > >>> > >> > >> There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and > >> qemuDomainObjPrivateXMLParse in order to handle the restart scenario. > >> > >> The rest of this looks OK, but do you need the Format/Parse logic for > >> the bitmap? > > > > No. The migration capabilities are rechecked every time libvirt connects > > to QEMU as said in the commit message and in qemu_domain.h: > > > > OK, so to be official... > > Reviewed-by: John Ferlan I pushed this series. Thanks for the review. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/14] Never ending story of user supplied aliases
On Fri, Oct 20, 2017 at 01:20:20PM +0200, Michal Privoznik wrote: > On 10/19/2017 03:33 PM, Pavel Hrdina wrote: > > On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote: > >> As discussed earlier [1], we should allow users to set device > >> aliases at the define time. While the discussed approach calls > >> for generating missing aliases too, I'm saving that for another > >> patch set. There are couple of reasons for that: > >> > >> a) I don't think it's really necessary (if users are interested > >>in a device they can just set the alias). > >> > >> b) This patch set is already big enough as is. > >> > >> c) Generating aliases is going to have its own problems. > >> > >> Therefore, for now I'm only proposing parsing user supplied > >> aliases. The idea is that it's not enabled by default for all > >> drivers. Any driver that want to/can provide this feature has to > >> set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some > >> drivers that don't have notion of device aliases. But the code is > >> generic enough so that it should be easy to use in the drivers > >> that do know what aliases are. > > > > This patch series missed some of the important parts of code > > that relies on our generated aliases: > > > > 1. qemuGetNextChrDevIndex() ... this will fail while attaching new > >chardev device without an alias if there is one already provided > >by user and doesn't match the one that we generate. > > > > 2. qemuAssignDeviceRedirdevAlias() ... same as 1) > > > > 3. qemuAssignDeviceShmemAlias() ... same as 1) > > Okay, these are easy to resolve. > > > > > 4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging > >network interface with user alias will fail because the alias is used > >to figure out VLAN of the interface. > > This one. Well, this function is called only for ancient QEMUs (those > which lack QMP, and even not for all of them). So do we care? Sure, I > can cripple my code and allow user aliases only if running sufficiently > new QEMU. Or just shrug and walk away. In that case just ignore it, it will print an error message and since this will affect only hot-plug of network devices with user alias specified we can claim that this operation is unsupported. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On 20.10.2017 13:09, Marc Hartmayer wrote: > On Thu, Oct 12, 2017 at 02:07 PM +0200, Jiri Denemark > wrote: >> On Mon, Oct 09, 2017 at 10:16:48 +0200, Marc Hartmayer wrote: >>> On Thu, Oct 05, 2017 at 02:11 PM +0200, Jiri Denemark >>> wrote: But it's going to be a bit complicated because we ask QEMU what the host CPU is and the interface we used would need to be enhanced to give us different results for all supported machine types so that we can select the right one when a domain is started. >>> >>> So how do we deal with this? >> >> I think we need to start discussing this on qemu-devel list. If I >> remember correctly, David Hildenbrand is the original author of the >> query-cpu-model-expansion QMP command. >> >> Please, Cc me so that the thread is more visible to me. >> >> Thanks, >> >> Jirka >> > > Hi all, > > we recently encountered the problem that the 'host-model' [1] has to be > related to the machine type of a domain. We have following problem: > >Let's assume we've a z13 system with a QEMU 2.9 and we define a >domain using the default s390-virtio-ccw machine together with the >host-model CPU mode [1]. The definition will have the machine >expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode >in the domain definition. In a next step we upgrade to QEMU 2.10 >(first version to recognize z14). Everything is still fine, even >though the machine runs in 2.9 compatibility mode. Finally we >upgrade to a z14. As a consequence it is not possible to start the >domain anymore as the machine type doesn't support our CPU host >model (which is expanded at start time of the domain). Yes, the host model may vary depending on QEMU version and to some degree also on compatibility machines (although I always try to push people to not introduce any new such stuff). Quoting for the libvirt documentation: https://libvirt.org/formatdomain.html "host-model The host-model mode is essentially a shortcut to copying host CPU definition from capabilities XML into domain XML. Since the CPU definition is copied just before starting a domain, exactly the same XML can be used on different hosts while still providing the best guest CPU each host supports." You're telling me that that copying does not happen, which seems to be the problematic part about this in my opinion. So I am not sure if probing anything else (as you noted below) is the correct thing to do. Copy it and you have a migration_safe and even static version of the _current_ host CPU. > > For determining the actual host-model the QMP command > 'query-cpu-model-expansion' is used. This is done once per QEMU binary > and the result of it is cached by libvirt. The problem with that is > that libvirt queries with the newest machine type of the QEMU binary > for the host CPU model. But now we realized that we have to consider > the machine type of each domain for the determination of the host CPU > model of a domain. > > We could now either probe the host CPU model for each QEMU binary + > machine type combination and for this we've to start a new QEMU > process each time. Or we can add a QMP command/parameter that allows > us to determine the host CPU model(s) with respect to the passed > machine type and/or all supported machine types. > > The QMP command query-cpu-model-expansion is currently described in > qapi-schema.json as follows: > > > ## > # @query-cpu-model-expansion: > # > # Expands a given CPU model (or a combination of CPU model + additional > options) > # to different granularities, allowing tooling to get an understanding what a > # specific CPU model looks like in QEMU under a certain configuration. > # > # This interface can be used to query the "host" CPU model. > # > # The data returned by this command may be affected by: > # > # * QEMU version: CPU models may look different depending on the QEMU version. > # (Except for CPU models reported as "static" in query-cpu-definitions.) > # * machine-type: CPU model may look different depending on the machine-type. > # (Except for CPU models reported as "static" in query-cpu-definitions.) > # * machine options (including accelerator): in some architectures, CPU models > # may look different depending on machine and accelerator options. (Except for > # CPU models reported as "static" in query-cpu-definitions.) > # * "-cpu" arguments and global properties: arguments to the -cpu option and > # global properties may affect expansion of CPU models. Using > # query-cpu-model-expansion while using these is not advised. > # > # Some architectures may not support all expansion types. s390x supports > # "full" and "static". > # > # Returns: a CpuModelExpansionInfo. Returns an error if expanding CPU models > is > # not supported, if the model cannot be expanded, if the model contains > # an unknown CPU definition name, unknown properties or properties > # with a wrong type. Also returns an error if an expansion type is > # n
[libvirt] [jenkins-ci PATCH 0/2] guests: Minor fixes and tweaks
1/2 is a bug fix, 2/2 a small improvement. Andrea Bolognani (2): guests: Reorder configuration steps for root authentication guests: Don't warn when bootstrapping package manager guests/tasks/base.yml | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 1/2] guests: Reorder configuration steps for root authentication
Key-based SSH authentication for root should be enabled before
changing the password, because if that fails (for example because
the user hasn't generated an SSH key pair yet) having changed the
root password will result in subsequent 'lcitool prepare' runs
failing to access the guest.
Signed-off-by: Andrea Bolognani
---
guests/tasks/base.yml | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml
index b220bb0..acdcc11 100644
--- a/guests/tasks/base.yml
+++ b/guests/tasks/base.yml
@@ -96,18 +96,18 @@
hostname:
name: '{{ inventory_hostname }}'
-- name: Configure root password and shell
- user:
-name: root
-password: '{{ lookup("file", lookup("env", "HOME") +
"/.config/lcitool/.root-password.hash") }}'
-shell: '{{ bash }}'
-
- name: Configure ssh access for the root user
authorized_key:
user: root
key: '{{ lookup("file", lookup("env", "HOME") + "/.ssh/id_rsa.pub") }}'
state: present
+- name: Configure root password and shell
+ user:
+name: root
+password: '{{ lookup("file", lookup("env", "HOME") +
"/.config/lcitool/.root-password.hash") }}'
+shell: '{{ bash }}'
+
- name: Disable password authentication for the root user
lineinfile:
path: /etc/ssh/sshd_config
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 2/2] guests: Don't warn when bootstrapping package manager
Ansible will try to get us to use the apt or dnf modules, but we can't really do that when we are bootstrapping said modules, so just silence the warning. Signed-off-by: Andrea Bolognani --- guests/tasks/base.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml index acdcc11..6acd967 100644 --- a/guests/tasks/base.yml +++ b/guests/tasks/base.yml @@ -37,6 +37,7 @@ command: apt-get install -y python-apt args: creates: /usr/lib/python2*/*-packages/apt +warn: no when: - package_format == 'deb' @@ -44,6 +45,7 @@ command: dnf install -y python2-dnf args: creates: /usr/lib*/python2*/*-packages/dnf +warn: no when: - os_name == 'Fedora' -- 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Question regarding the validity of QEMU capabilities
On Fri, Oct 20, 2017 at 13:15:41 +0200, Marc Hartmayer wrote: > On Fri, Oct 20, 2017 at 01:10 PM +0200, Pavel Hrdina > wrote: > > On Fri, Oct 20, 2017 at 12:59:51PM +0200, Marc Hartmayer wrote: > >> Hi all, > >> > >> the actual capabilities of QEMU are depending on the host. This > >> includes dependencies like which kernel modules are loaded or which > >> kernel parameters are used (e.g. kvm.nested). Therefore, after a > >> restart we cannot be sure that the QEMU capabilities remain the same. > >> > >> How can we solve this problem? > > > > Hi, > > > > thanks for bringing up this issue, we kind of already know about it > > but it's good idea to discuss it publicly. > > > >> > >> I have come up with two ways: > >> - reprobe the capabilities with every host reboot > > > > This is the solution that was agreed on but nobody was motivated enough > > to write the code :). > > First - thank you for the quick answer :) > > Would it be okay if the solution depends on systemd? Not really. It may of course use systemd if it's present, but it should work even on systems without systemd. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/14] Never ending story of user supplied aliases
On 10/19/2017 03:33 PM, Pavel Hrdina wrote: > On Thu, Oct 19, 2017 at 10:10:55AM +0200, Michal Privoznik wrote: >> As discussed earlier [1], we should allow users to set device >> aliases at the define time. While the discussed approach calls >> for generating missing aliases too, I'm saving that for another >> patch set. There are couple of reasons for that: >> >> a) I don't think it's really necessary (if users are interested >>in a device they can just set the alias). >> >> b) This patch set is already big enough as is. >> >> c) Generating aliases is going to have its own problems. >> >> Therefore, for now I'm only proposing parsing user supplied >> aliases. The idea is that it's not enabled by default for all >> drivers. Any driver that want to/can provide this feature has to >> set VIR_DOMAIN_DEF_FEATURE_USER_ALIAS. Note that we have some >> drivers that don't have notion of device aliases. But the code is >> generic enough so that it should be easy to use in the drivers >> that do know what aliases are. > > This patch series missed some of the important parts of code > that relies on our generated aliases: > > 1. qemuGetNextChrDevIndex() ... this will fail while attaching new >chardev device without an alias if there is one already provided >by user and doesn't match the one that we generate. > > 2. qemuAssignDeviceRedirdevAlias() ... same as 1) > > 3. qemuAssignDeviceShmemAlias() ... same as 1) Okay, these are easy to resolve. > > 4. qemuDomainNetVLAN() ... similar to the previous ones, hot-plugging >network interface with user alias will fail because the alias is used >to figure out VLAN of the interface. This one. Well, this function is called only for ancient QEMUs (those which lack QMP, and even not for all of them). So do we care? Sure, I can cripple my code and allow user aliases only if running sufficiently new QEMU. Or just shrug and walk away. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Question about the host-model CPU mode
On Thu, Oct 12, 2017 at 02:07 PM +0200, Jiri Denemark
wrote:
> On Mon, Oct 09, 2017 at 10:16:48 +0200, Marc Hartmayer wrote:
>> On Thu, Oct 05, 2017 at 02:11 PM +0200, Jiri Denemark
>> wrote:
>> > But it's going to be a bit complicated because we ask QEMU what the
>> > host CPU is and the interface we used would need to be enhanced to
>> > give us different results for all supported machine types so that we
>> > can select the right one when a domain is started.
>>
>> So how do we deal with this?
>
> I think we need to start discussing this on qemu-devel list. If I
> remember correctly, David Hildenbrand is the original author of the
> query-cpu-model-expansion QMP command.
>
> Please, Cc me so that the thread is more visible to me.
>
> Thanks,
>
> Jirka
>
Hi all,
we recently encountered the problem that the 'host-model' [1] has to be
related to the machine type of a domain. We have following problem:
Let's assume we've a z13 system with a QEMU 2.9 and we define a
domain using the default s390-virtio-ccw machine together with the
host-model CPU mode [1]. The definition will have the machine
expanded to s390-virtio-ccw-2.9 but retain the host-model CPU mode
in the domain definition. In a next step we upgrade to QEMU 2.10
(first version to recognize z14). Everything is still fine, even
though the machine runs in 2.9 compatibility mode. Finally we
upgrade to a z14. As a consequence it is not possible to start the
domain anymore as the machine type doesn't support our CPU host
model (which is expanded at start time of the domain).
For determining the actual host-model the QMP command
'query-cpu-model-expansion' is used. This is done once per QEMU binary
and the result of it is cached by libvirt. The problem with that is
that libvirt queries with the newest machine type of the QEMU binary
for the host CPU model. But now we realized that we have to consider
the machine type of each domain for the determination of the host CPU
model of a domain.
We could now either probe the host CPU model for each QEMU binary +
machine type combination and for this we've to start a new QEMU
process each time. Or we can add a QMP command/parameter that allows
us to determine the host CPU model(s) with respect to the passed
machine type and/or all supported machine types.
The QMP command query-cpu-model-expansion is currently described in
qapi-schema.json as follows:
##
# @query-cpu-model-expansion:
#
# Expands a given CPU model (or a combination of CPU model + additional options)
# to different granularities, allowing tooling to get an understanding what a
# specific CPU model looks like in QEMU under a certain configuration.
#
# This interface can be used to query the "host" CPU model.
#
# The data returned by this command may be affected by:
#
# * QEMU version: CPU models may look different depending on the QEMU version.
# (Except for CPU models reported as "static" in query-cpu-definitions.)
# * machine-type: CPU model may look different depending on the machine-type.
# (Except for CPU models reported as "static" in query-cpu-definitions.)
# * machine options (including accelerator): in some architectures, CPU models
# may look different depending on machine and accelerator options. (Except for
# CPU models reported as "static" in query-cpu-definitions.)
# * "-cpu" arguments and global properties: arguments to the -cpu option and
# global properties may affect expansion of CPU models. Using
# query-cpu-model-expansion while using these is not advised.
#
# Some architectures may not support all expansion types. s390x supports
# "full" and "static".
#
# Returns: a CpuModelExpansionInfo. Returns an error if expanding CPU models is
# not supported, if the model cannot be expanded, if the model contains
# an unknown CPU definition name, unknown properties or properties
# with a wrong type. Also returns an error if an expansion type is
# not supported.
#
# Since: 2.8.0
##
{ 'command': 'query-cpu-model-expansion',
'data': { 'type': 'CpuModelExpansionType',
'model': 'CpuModelInfo' },
'returns': 'CpuModelExpansionInfo' }
4:46:25 PM
◄
qapi-schema.json
What do you think?
Thank you.
Marc
[1] https://libvirt.org/formatdomain.html#elementsCPU
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Question regarding the validity of QEMU capabilities
On Fri, Oct 20, 2017 at 01:10 PM +0200, Pavel Hrdina wrote: > On Fri, Oct 20, 2017 at 12:59:51PM +0200, Marc Hartmayer wrote: >> Hi all, >> >> the actual capabilities of QEMU are depending on the host. This >> includes dependencies like which kernel modules are loaded or which >> kernel parameters are used (e.g. kvm.nested). Therefore, after a >> restart we cannot be sure that the QEMU capabilities remain the same. >> >> How can we solve this problem? > > Hi, > > thanks for bringing up this issue, we kind of already know about it > but it's good idea to discuss it publicly. > >> >> I have come up with two ways: >> - reprobe the capabilities with every host reboot > > This is the solution that was agreed on but nobody was motivated enough > to write the code :). First - thank you for the quick answer :) Would it be okay if the solution depends on systemd? > >> - check for every possible change in virQEMUCapsIsValid... (this is >>already done for KVM). In my opinion this is not the way to go. > > Pavel -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Question regarding the validity of QEMU capabilities
On Fri, Oct 20, 2017 at 12:59:51PM +0200, Marc Hartmayer wrote: > Hi all, > > the actual capabilities of QEMU are depending on the host. This > includes dependencies like which kernel modules are loaded or which > kernel parameters are used (e.g. kvm.nested). Therefore, after a > restart we cannot be sure that the QEMU capabilities remain the same. > > How can we solve this problem? Hi, thanks for bringing up this issue, we kind of already know about it but it's good idea to discuss it publicly. > > I have come up with two ways: > - reprobe the capabilities with every host reboot This is the solution that was agreed on but nobody was motivated enough to write the code :). > - check for every possible change in virQEMUCapsIsValid... (this is >already done for KVM). In my opinion this is not the way to go. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] qemu: Store supported migration capabilities in a bitmap
On 10/20/2017 03:03 AM, Jiri Denemark wrote:
> On Thu, Oct 19, 2017 at 18:30:53 -0400, John Ferlan wrote:
>>
>>
>> On 10/18/2017 07:29 AM, Jiri Denemark wrote:
>>> Each time we need to check whether a given migration capability is
>>> supported by QEMU, we call query-migrate-capabilities QMP command and
>>> lookup the capability in the returned list. Asking for the list of
>>> supported capabilities once when we connect to QEMU and storing the
>>> result in a bitmap is much better and we don't need to enter a monitor
>>> just to check whether a migration capability is supported.
>>>
>>> Signed-off-by: Jiri Denemark
>>> ---
>>> src/qemu/qemu_domain.c | 68
>>> +
>>> src/qemu/qemu_domain.h | 9 +++
>>> src/qemu/qemu_process.c | 13 +-
>>> 3 files changed, 78 insertions(+), 12 deletions(-)
>>>
>>
>> There's no corresponding adjustment to qemuDomainObjPrivateXMLFormat and
>> qemuDomainObjPrivateXMLParse in order to handle the restart scenario.
>>
>> The rest of this looks OK, but do you need the Format/Parse logic for
>> the bitmap?
>
> No. The migration capabilities are rechecked every time libvirt connects
> to QEMU as said in the commit message and in qemu_domain.h:
>
OK, so to be official...
Reviewed-by: John Ferlan
John
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 5201c6a0ac..fb20d8ea63 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -323,6 +323,10 @@ struct _qemuDomainObjPrivate {
>>>
>>> /* Tracks blockjob state for vm. Valid only while reconnecting to
>>> qemu. */
>>> virTristateBool reconnectBlockjobs;
>>> +
>>> +/* Migration capabilities. Rechecked on reconnect, not to be saved in
>>> + * private XML. */
>>> +virBitmapPtr migrationCaps;
>>> };
>
> Jirka
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] Question regarding the validity of QEMU capabilities
Hi all, the actual capabilities of QEMU are depending on the host. This includes dependencies like which kernel modules are loaded or which kernel parameters are used (e.g. kvm.nested). Therefore, after a restart we cannot be sure that the QEMU capabilities remain the same. How can we solve this problem? I have come up with two ways: - reprobe the capabilities with every host reboot - check for every possible change in virQEMUCapsIsValid... (this is already done for KVM). In my opinion this is not the way to go. Thanks. Marc -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] qemu: Use switch in qemuMigrationCompleted
On Fri, Oct 20, 2017 at 12:12 PM +0200, Jiri Denemark
wrote:
> On Fri, Oct 20, 2017 at 11:28:37 +0200, Marc Hartmayer wrote:
>> On Thu, Oct 19, 2017 at 03:56 PM +0200, Jiri Denemark
>> wrote:
>> > When adding a new job state it's useful to let the compiler complain
>> > about places where we need to think about what to do with the new
>> > state.
>> >
>> > Signed-off-by: Jiri Denemark
>> > ---
>> > src/qemu/qemu_migration.c | 23 ++-
>> > 1 file changed, 18 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> > index 72edbb667..c3f9c38b2 100644
>> > --- a/src/qemu/qemu_migration.c
>> > +++ b/src/qemu/qemu_migration.c
>> > @@ -1531,18 +1531,31 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
>> > return 0;
>> >
>> > error:
>> > -/* state can not be active or completed at this point */
>> > -if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
>> > -jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
>> > +switch (jobInfo->status) {
>> > +case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
>> > +case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
>> > /* The migration was aborted by us rather than QEMU itself. */
>> > jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
>> > return -2;
>> > -} else if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED) {
>> > +
>> > +case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
>> > +/* Something failed after QEMU already finished the migration. */
>> > jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
>> > return -1;
>> > -} else {
>> > +
>> > +case QEMU_DOMAIN_JOB_STATUS_FAILED:
>> > +case QEMU_DOMAIN_JOB_STATUS_CANCELED:
>> > +/* QEMU aborted the migration. */
>> > return -1;
>> > +
>> > +case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
>> > +case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
>> > +case QEMU_DOMAIN_JOB_STATUS_NONE:
>> > +/* Impossible. */
>> > +break;
>> > }
>> > +
>> > +return -1;
>> > }
>>
>> I think you have to add ATTRIBUTE_FALLTHROUGH for the intended
>> fallthroughs (e.g. for gcc 7).
>
> This should only be needed if there was any code between the cases.
Thanks for the information.
>
> Jirka
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] qemu: Use switch in qemuMigrationCompleted
On Fri, Oct 20, 2017 at 11:28:37 +0200, Marc Hartmayer wrote:
> On Thu, Oct 19, 2017 at 03:56 PM +0200, Jiri Denemark
> wrote:
> > When adding a new job state it's useful to let the compiler complain
> > about places where we need to think about what to do with the new
> > state.
> >
> > Signed-off-by: Jiri Denemark
> > ---
> > src/qemu/qemu_migration.c | 23 ++-
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 72edbb667..c3f9c38b2 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -1531,18 +1531,31 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
> > return 0;
> >
> > error:
> > -/* state can not be active or completed at this point */
> > -if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
> > -jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
> > +switch (jobInfo->status) {
> > +case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
> > +case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
> > /* The migration was aborted by us rather than QEMU itself. */
> > jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
> > return -2;
> > -} else if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED) {
> > +
> > +case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
> > +/* Something failed after QEMU already finished the migration. */
> > jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
> > return -1;
> > -} else {
> > +
> > +case QEMU_DOMAIN_JOB_STATUS_FAILED:
> > +case QEMU_DOMAIN_JOB_STATUS_CANCELED:
> > +/* QEMU aborted the migration. */
> > return -1;
> > +
> > +case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
> > +case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
> > +case QEMU_DOMAIN_JOB_STATUS_NONE:
> > +/* Impossible. */
> > +break;
> > }
> > +
> > +return -1;
> > }
>
> I think you have to add ATTRIBUTE_FALLTHROUGH for the intended
> fallthroughs (e.g. for gcc 7).
This should only be needed if there was any code between the cases.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] qemu: Use switch in qemuMigrationCompleted
On Thu, Oct 19, 2017 at 03:56 PM +0200, Jiri Denemark
wrote:
> When adding a new job state it's useful to let the compiler complain
> about places where we need to think about what to do with the new
> state.
>
> Signed-off-by: Jiri Denemark
> ---
> src/qemu/qemu_migration.c | 23 ++-
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 72edbb667..c3f9c38b2 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1531,18 +1531,31 @@ qemuMigrationCompleted(virQEMUDriverPtr driver,
> return 0;
>
> error:
> -/* state can not be active or completed at this point */
> -if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_MIGRATING ||
> -jobInfo->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) {
> +switch (jobInfo->status) {
> +case QEMU_DOMAIN_JOB_STATUS_MIGRATING:
> +case QEMU_DOMAIN_JOB_STATUS_POSTCOPY:
> /* The migration was aborted by us rather than QEMU itself. */
> jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
> return -2;
> -} else if (jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED) {
> +
> +case QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED:
> +/* Something failed after QEMU already finished the migration. */
> jobInfo->status = QEMU_DOMAIN_JOB_STATUS_FAILED;
> return -1;
> -} else {
> +
> +case QEMU_DOMAIN_JOB_STATUS_FAILED:
> +case QEMU_DOMAIN_JOB_STATUS_CANCELED:
> +/* QEMU aborted the migration. */
> return -1;
> +
> +case QEMU_DOMAIN_JOB_STATUS_ACTIVE:
> +case QEMU_DOMAIN_JOB_STATUS_COMPLETED:
> +case QEMU_DOMAIN_JOB_STATUS_NONE:
> +/* Impossible. */
> +break;
> }
> +
> +return -1;
> }
I think you have to add ATTRIBUTE_FALLTHROUGH for the intended
fallthroughs (e.g. for gcc 7).
>
>
> --
> 2.14.2
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [RFC 0/6] enable numa configuration before machine_init() from HMP/QMP
On Thu, Oct 19, 2017 at 05:56:49PM -0200, Eduardo Habkost wrote: > On Thu, Oct 19, 2017 at 04:28:59PM +0100, Daniel P. Berrange wrote: > > On Thu, Oct 19, 2017 at 11:21:22AM -0400, Igor Mammedov wrote: > > > - Original Message - > > > > From: "Daniel P. Berrange" > > > > To: "Igor Mammedov" > > > > Cc: "peter maydell" , pkre...@redhat.com, > > > > ehabk...@redhat.com, coh...@redhat.com, > > > > qemu-de...@nongnu.org, arm...@redhat.com, pbonz...@redhat.com, > > > > da...@gibson.dropbear.id.au > > > > Sent: Wednesday, October 18, 2017 5:30:10 PM > > > > Subject: Re: [Qemu-devel] [RFC 0/6] enable numa configuration before > > > > machine_init() from HMP/QMP > > > > > > > > On Tue, Oct 17, 2017 at 06:06:35PM +0200, Igor Mammedov wrote: > > > > > On Tue, 17 Oct 2017 16:07:59 +0100 > > > > > "Daniel P. Berrange" wrote: > > > > > > > > > > > On Tue, Oct 17, 2017 at 09:27:02AM +0200, Igor Mammedov wrote: > > > > > > > On Mon, 16 Oct 2017 17:36:36 +0100 > > > > > > > "Daniel P. Berrange" wrote: > > > > > > > > > > > > > > > On Mon, Oct 16, 2017 at 06:22:50PM +0200, Igor Mammedov wrote: > > > > > > > > > Series allows to configure NUMA mapping at runtime using > > > > > > > > > QMP/HMP > > > > > > > > > interface. For that to happen it introduces a new '-paused' > > > > > > > > > CLI > > > > > > > > > option > > > > > > > > > which allows to pause QEMU before machine_init() is run and > > > > > > > > > adds new set-numa-node HMP/QMP commands which in conjuction > > > > > > > > > with > > > > > > > > > info hotpluggable-cpus/query-hotpluggable-cpus allow to > > > > > > > > > configure > > > > > > > > > NUMA mapping for cpus. > > > > > > > > > > > > > > > > What's the problem we're seeking solve here compared to what we > > > > > > > > currently > > > > > > > > do for NUMA configuration ? > > > > > > > From RHBZ1382425 > > > > > > > " > > > > > > > Current -numa CLI interface is quite limited in terms that allow > > > > > > > map > > > > > > > CPUs to NUMA nodes as it requires to provide cpu_index values > > > > > > > which > > > > > > > are non obvious and depend on machine/arch. As result libvirt has > > > > > > > to > > > > > > > assume/re-implement cpu_index allocation logic to provide valid > > > > > > > values for -numa cpus=... QEMU CLI option. > > > > > > > > > > > > In broad terms, this problem applies to every device / object > > > > > > libvirt > > > > > > asks QEMU to create. For everything else libvirt is able to assign a > > > > > > "id" string, which is can then use to identify the thing later. The > > > > > > CPU stuff is different because libvirt isn't able to provide 'id' > > > > > > strings for each CPU - QEMU generates a psuedo-id internally which > > > > > > libvirt has to infer. The latter is the same problem we had with > > > > > > devices before '-device' was introduced allowing 'id' naming. > > > > > > > > > > > > IMHO we should take the same approach with CPUs and start modelling > > > > > > the individual CPUs as something we can explicitly create with > > > > > > -object > > > > > > or -device. That way libvirt can assign names and does not have to > > > > > > care about CPU index values, and it all works just the same way as > > > > > > any other devices / object we create > > > > > > > > > > > > ie instead of: > > > > > > > > > > > > -smp 8,sockets=4,cores=2,threads=1 > > > > > > -numa node,nodeid=0,cpus=0-3 > > > > > > -numa node,nodeid=1,cpus=4-7 > > > > > > > > > > > > we could do: > > > > > > > > > > > > -object numa-node,id=numa0 > > > > > > -object numa-node,id=numa1 > > > > > > -object cpu,id=cpu0,node=numa0,socket=0,core=0,thread=0 > > > > > > -object cpu,id=cpu1,node=numa0,socket=0,core=1,thread=0 > > > > > > -object cpu,id=cpu2,node=numa0,socket=1,core=0,thread=0 > > > > > > -object cpu,id=cpu3,node=numa0,socket=1,core=1,thread=0 > > > > > > -object cpu,id=cpu4,node=numa1,socket=2,core=0,thread=0 > > > > > > -object cpu,id=cpu5,node=numa1,socket=2,core=1,thread=0 > > > > > > -object cpu,id=cpu6,node=numa1,socket=3,core=0,thread=0 > > > > > > -object cpu,id=cpu7,node=numa1,socket=3,core=1,thread=0 > > > > > the follow up question would be where do "socket=3,core=1,thread=0" > > > > > come from, currently these options are the function of > > > > > (-M foo -smp ...) and can be queried vi query-hotpluggble-cpus at > > > > > runtime after qemu parses -M and -smp options. > > > > > > > > NB, I realize my example was open to mis-interpretation. The values I'm > > > > illustrating here for socket=3,core=1,thread=0 and *not* ID values, they > > > > are a plain enumeration of values. ie this is saying the 4th socket, the > > > > 2nd core and the 1st thread. Internally QEMU might have the 2nd core > > > > with a core-id of 8, or 7038 or whatever architecture specific numbering > > > > scheme makes sense, but that's not what the mgmt app gives at the CLI > > > > level > > > Even though fixed properties/values simplicity is tem
