Any objections to retiring the v0.9.6-maint branch? After all, we have
already retired the v0.9.11-maint branch
(http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
was the backport of a single CVE fix. The branch no longer builds
cleanly on Fedora 20, and while I could identify patches to backport to
fix the build situation, it's not worth my time if we can just retire
the branch.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/09/2014 07:40 PM, John Ferlan wrote:
> The "rawio" setting for a lun isn't duplicated
> in the scsi_host hostdev device for a lun. Rather than "requiring" some
> disk in the disk list to add the rawio, add the "rawio" property to
> the scsi_host hostdev lun.
>
> John Ferlan (3):
> hostdev: Add "rawio" attribute to _virDomainHostdevSubsysSCSI
> qemu: Process the hostdev "rawio" setting
> domain_conf: Process the "rawio" for a hostdev device
>
> docs/formatdomain.html.in | 12 ++--
> docs/schemas/domaincommon.rng | 18 +++
> src/conf/domain_conf.c | 31 +++
> src/conf/domain_conf.h | 2 ++
> src/qemu/qemu_domain.c | 21 +
> src/qemu/qemu_domain.h | 4 +++
> src/qemu/qemu_driver.c | 1 +
> src/qemu/qemu_process.c| 20 -
> .../qemuxml2argv-hostdev-scsi-rawio.xml| 35
> ++
> tests/qemuxml2xmltest.c| 1 +
> 10 files changed, 135 insertions(+), 10 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml
>
ping?
tks
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/14 00:29, John Ferlan wrote:
>
>
> On 09/11/2014 01:47 PM, Peter Krempa wrote:
>> Now that we have a simple function to check locality of storage, reuse
>> it in qemuDomainCheckDiskPresence().
>>
>> Also reuse check for empty storage source.
>> ---
>> src/qemu/qemu_domain.c | 8 +++-
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>
>
> ACK
Thanks, I've currently pushed 2/6 and 6/6 and I'll try to implement your
feedback on the rest later and I'll possibly repost the series if I find
the changes were not trivial.
>
> John
>
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
While doing some investigation for another bug I found that I could
not qemu-attach to the process and got the following:
error: Operation not supported: JSON monitor is required
while running thru qemuProcessAttach. Since we can only get the data
using the JSON parser and if the guest to be attached to doesn't have
it we shouldn't just fail. See example in virsh qemu-attach for sample
command that failed.
Signed-off-by: John Ferlan
---
I also considered removing the call from qemuProcessAttach rather than
this approach.
src/qemu/qemu_monitor.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8927dbb..4342088 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
return -1;
}
-if (!mon->json) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("JSON monitor is required"));
-return -1;
-}
+/* Requires JSON to make the query */
+if (!mon->json)
+return 0;
return qemuMonitorJSONGetIOThreads(mon, iothreads);
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/14 18:31, Eric Blake wrote:
> On 09/17/2014 10:25 AM, Peter Krempa wrote:
>> Live definition was used to look up the disk index while persistent one
>> was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
>> correct def and report a nice error.
>>
>> Unfortunately it's accessible via read-only connection.
>>
>
> Mitigation - a read-only connection can only crash libvirtd in the cases
> where the guest is hot-plugging disks without reflecting those changes
> to the persistent definition. So avoiding hotplug, or doing hotplug
> where persistent is always modified alongside live definition, will
> avoid the out-of-bounds access.
I've added this paragraph to the commit message
>
>> Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
>> Reported-by: Luyao Huang
>> Signed-off-by: Peter Krempa
>> ---
>> src/qemu/qemu_driver.c | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> ACK. I can write up the libvirt security notice; we'll eventually need
> this backported to all the affected maint branches. I'll coordinate the
> backport effort with you on IRC.
>
and pushed this patch. I can handle the backport tomorrow if you don't
beat me to it.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/2014 10:25 AM, Peter Krempa wrote:
> Live definition was used to look up the disk index while persistent one
> was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
> correct def and report a nice error.
>
> Unfortunately it's accessible via read-only connection.
>
Mitigation - a read-only connection can only crash libvirtd in the cases
where the guest is hot-plugging disks without reflecting those changes
to the persistent definition. So avoiding hotplug, or doing hotplug
where persistent is always modified alongside live definition, will
avoid the out-of-bounds access.
> Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
> Reported-by: Luyao Huang
> Signed-off-by: Peter Krempa
> ---
> src/qemu/qemu_driver.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
ACK. I can write up the libvirt security notice; we'll eventually need
this backported to all the affected maint branches. I'll coordinate the
backport effort with you on IRC.
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a5a49ac..209c40e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -16317,9 +16317,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> }
>
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> -int idx = virDomainDiskIndexByName(vm->def, disk, true);
> -if (idx < 0)
> +int idx = virDomainDiskIndexByName(persistentDef, disk, true);
> +if (idx < 0) {
> +virReportError(VIR_ERR_INVALID_ARG,
> + _("disk '%s' was not found in the domain config"),
> + disk);
> goto endjob;
> +}
> reply = persistentDef->disks[idx]->blkdeviotune;
> }
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Live definition was used to look up the disk index while persistent one
was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
correct def and report a nice error.
Unfortunately it's accessible via read-only connection.
Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
Reported-by: Luyao Huang
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a5a49ac..209c40e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16317,9 +16317,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-int idx = virDomainDiskIndexByName(vm->def, disk, true);
-if (idx < 0)
+int idx = virDomainDiskIndexByName(persistentDef, disk, true);
+if (idx < 0) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("disk '%s' was not found in the domain config"),
+ disk);
goto endjob;
+}
reply = persistentDef->disks[idx]->blkdeviotune;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/14 00:37, Eric Blake wrote:
> On 09/15/2014 09:42 AM, Peter Krempa wrote:
>> From: Francesco Romani
>>
>> Management software wants to be able to allocate disk space on demand.
>> To support this they need keep track of the space occupation of the
>> block device. This information is reported by qemu as part of block
>> stats.
>>
>> This patch extend the block information in the bulk stats with the
>> allocation information.
>>
>> To keep the same behaviour a helper is extracted from
>> qemuMonitorJSONGetBlockExtent in order to get per-device allocation
>> information.
>>
>> Signed-off-by: Francesco Romani
>> ---
>> src/libvirt.c| 2 +
>> src/qemu/qemu_driver.c | 18 +
>> src/qemu/qemu_monitor.h | 1 +
>> src/qemu/qemu_monitor_json.c | 91
>> ++--
>> 4 files changed, 92 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index ab10a3a..a8892a2 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
>> @@ -21654,6 +21654,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>> * unsigned long long.
>> * "block..errors" - Xen only: the 'oo_req' value as
>> *unsigned long long.
>> + * "block..allocation" - offset of the highest written sector
>> + *as unsigned long long.
>
> If we are repeating virDomainGetBlockInfo() here, I'd rather see us
> report all 3 pieces of information (allocation, capacity, physical)
> instead of just one.
>
>> +typedef enum {
>> +QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK,
>> +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT,
>> +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS,
>> +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET
>> +} qemuMonitorBlockExtentError;
>
> We require C99, so please have a trailing comma in the enum (it makes
> adding new elements easier, because they don't have to modify an
> existing line).
>
Thanks for the reviews, I've pushed this series with the changes you've
suggested except this patch as either I or Francesco will add the
requested fields.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/2014 09:52 AM, Daniel P. Berrange wrote:
>
> Ah, so this is why you shouldn't take the precise solution requested in
> a bug too literally, and instead look at the general picture :-)
>
> So QEMU exposes alot of stuff:
>
> $ qemu-kvm -device virtio-net,?
> virtio-net-pci.guest_tso4=on/off
> virtio-net-pci.guest_tso6=on/off
> virtio-net-pci.guest_ecn=on/off
> virtio-net-pci.guest_ufo=on/off
> virtio-net-pci.host_tso4=on/off
> virtio-net-pci.host_tso6=on/off
> virtio-net-pci.host_ecn=on/off
> virtio-net-pci.host_ufo=on/off
>
> Which to me indicates that Eric's suggestion for sub-elements is a
Wasn't my idea, but Jan's.
> good idea. eg go for:
>
>
>
>
>
>
> and support the host bits too
but yes, I could live with that, now that I'm seeing how many more
options there are to be exposed.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Sep 17, 2014 at 05:36:18PM +0200, Ján Tomko wrote:
> On 09/17/2014 04:57 PM, Daniel P. Berrange wrote:
> > On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
> >> On 09/11/2014 05:43 AM, Ján Tomko wrote:
> >>> Add the following attributes:
> >>> csum, gso, guest_tso4, guest_tso6, guest_ecn
> >>> to the element of network interface
> >>> which control the virtio-net device properties
> >>> of the same names.
> >>> ---
> >>> docs/formatdomain.html.in | 27
> >>> docs/schemas/domaincommon.rng | 25 +++
> >>> src/conf/domain_conf.c | 81
> >>> ++
> >>> src/conf/domain_conf.h | 5 ++
> >>> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +
> >>> tests/qemuxml2xmltest.c| 1 +
> >>> 6 files changed, 171 insertions(+)
> >>> create mode 100644
> >>> tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
> >>>
> >>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> >>> index a2ea758..5b2758a 100644
> >>> --- a/docs/formatdomain.html.in
> >>> +++ b/docs/formatdomain.html.in
> >>> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
> >>>
> >>> >>> event_idx='off' queues='5'/>
> >>>
> >>> +
> >>> +
> >>> +
> >>> +
> >>> + >>> guest_tso6='off' guest_ecn='off'/>
> >>> +
> >>
> >> Are we stuck with names with underscores in our XML? I'm still not sure
> >> if we've come up with the best naming for exposing all these knobs.
> >
> > I'm not really convinced having a 'guest_' prefix really buys
> > us anything here, since there's no naming clash to avoid. Why
> > don't we just kill the 'guest_' prefixes.
>
> The clash is in the options I didn't expose:
> http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h=6ceb5aa92
>
> because they weren't requested by the (private :() bug 1139364
Ah, so this is why you shouldn't take the precise solution requested in
a bug too literally, and instead look at the general picture :-)
So QEMU exposes alot of stuff:
$ qemu-kvm -device virtio-net,?
virtio-net-pci.ioeventfd=on/off
virtio-net-pci.vectors=uint32
virtio-net-pci.indirect_desc=on/off
virtio-net-pci.event_idx=on/off
virtio-net-pci.any_layout=on/off
virtio-net-pci.csum=on/off
virtio-net-pci.guest_csum=on/off
virtio-net-pci.gso=on/off
virtio-net-pci.guest_tso4=on/off
virtio-net-pci.guest_tso6=on/off
virtio-net-pci.guest_ecn=on/off
virtio-net-pci.guest_ufo=on/off
virtio-net-pci.host_tso4=on/off
virtio-net-pci.host_tso6=on/off
virtio-net-pci.host_ecn=on/off
virtio-net-pci.host_ufo=on/off
virtio-net-pci.mrg_rxbuf=on/off
virtio-net-pci.status=on/off
virtio-net-pci.ctrl_vq=on/off
virtio-net-pci.ctrl_rx=on/off
virtio-net-pci.ctrl_vlan=on/off
virtio-net-pci.ctrl_rx_extra=on/off
virtio-net-pci.ctrl_mac_addr=on/off
virtio-net-pci.ctrl_guest_offloads=on/off
virtio-net-pci.mq=on/off
virtio-net-pci.mac=macaddr
virtio-net-pci.vlan=vlan
virtio-net-pci.netdev=netdev
virtio-net-pci.bootindex=int32
virtio-net-pci.x-txtimer=uint32
virtio-net-pci.x-txburst=int32
virtio-net-pci.tx=str
virtio-net-pci.addr=pci-devfn
virtio-net-pci.romfile=str
virtio-net-pci.rombar=uint32
virtio-net-pci.multifunction=on/off
virtio-net-pci.command_serr_enable=on/off
Which to me indicates that Eric's suggestion for sub-elements is a
good idea. eg go for:
and support the host bits too
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 17.09.2014 17:40, Martin Kletzander wrote:
On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote:
As of f05b6a918e28 the test produces the list of paths that can
be passed to and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems. Therefore, the path - if missing - is
added to pretend it's there.
Signed-off-by: Michal Privoznik
---
tests/domaincapstest.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 8543963..067ad4d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
+
+/* Moreover, as of f05b6a918e28 we are expecting to see
+ * OVMF_CODE.fd file which may not exists everywhere. */
+if (!domCaps->os.loader.values.nvalues) {
+virDomainCapsLoaderPtr loader = &domCaps->os.loader;
+
+if (fillStringValues(&loader->values,
+ "/usr/share/OVMF/OVMF_CODE.fd",
+ NULL) < 0)
+return -1;
+}
return 0;
}
#endif /* WITH_QEMU */
--
1.8.5.5
ACK, build-breaker (at least for me).
Thank you, pushed now.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote:
As of f05b6a918e28 the test produces the list of paths that can
be passed to and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems. Therefore, the path - if missing - is
added to pretend it's there.
Signed-off-by: Michal Privoznik
---
tests/domaincapstest.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 8543963..067ad4d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
+
+/* Moreover, as of f05b6a918e28 we are expecting to see
+ * OVMF_CODE.fd file which may not exists everywhere. */
+if (!domCaps->os.loader.values.nvalues) {
+virDomainCapsLoaderPtr loader = &domCaps->os.loader;
+
+if (fillStringValues(&loader->values,
+ "/usr/share/OVMF/OVMF_CODE.fd",
+ NULL) < 0)
+return -1;
+}
return 0;
}
#endif /* WITH_QEMU */
--
1.8.5.5
ACK, build-breaker (at least for me).
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/2014 04:57 PM, Daniel P. Berrange wrote:
> On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
>> On 09/11/2014 05:43 AM, Ján Tomko wrote:
>>> Add the following attributes:
>>> csum, gso, guest_tso4, guest_tso6, guest_ecn
>>> to the element of network interface
>>> which control the virtio-net device properties
>>> of the same names.
>>> ---
>>> docs/formatdomain.html.in | 27
>>> docs/schemas/domaincommon.rng | 25 +++
>>> src/conf/domain_conf.c | 81
>>> ++
>>> src/conf/domain_conf.h | 5 ++
>>> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +
>>> tests/qemuxml2xmltest.c| 1 +
>>> 6 files changed, 171 insertions(+)
>>> create mode 100644
>>> tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index a2ea758..5b2758a 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
>>>
>>>>> event_idx='off' queues='5'/>
>>>
>>> +
>>> +
>>> +
>>> +
>>> + >> guest_ecn='off'/>
>>> +
>>
>> Are we stuck with names with underscores in our XML? I'm still not sure
>> if we've come up with the best naming for exposing all these knobs.
>
> I'm not really convinced having a 'guest_' prefix really buys
> us anything here, since there's no naming clash to avoid. Why
> don't we just kill the 'guest_' prefixes.
The clash is in the options I didn't expose:
http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h=6ceb5aa92
because they weren't requested by the (private :() bug 1139364
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
As of f05b6a918e28 the test produces the list of paths that can
be passed to and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems. Therefore, the path - if missing - is
added to pretend it's there.
Signed-off-by: Michal Privoznik
---
tests/domaincapstest.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 8543963..067ad4d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
+
+/* Moreover, as of f05b6a918e28 we are expecting to see
+ * OVMF_CODE.fd file which may not exists everywhere. */
+if (!domCaps->os.loader.values.nvalues) {
+virDomainCapsLoaderPtr loader = &domCaps->os.loader;
+
+if (fillStringValues(&loader->values,
+ "/usr/share/OVMF/OVMF_CODE.fd",
+ NULL) < 0)
+return -1;
+}
return 0;
}
#endif /* WITH_QEMU */
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[adding libvirt list]
On 09/17/2014 09:04 AM, Stefan Hajnoczi wrote:
> On Wed, Sep 17, 2014 at 10:25 AM, Paolo Bonzini wrote:
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> Il 17/09/2014 11:06, Stefan Hajnoczi ha scritto:
>>> I think the fundamental problem here is that the mirror block job
>>> on the source host does not synchronize with live migration.
>>>
>>> Remember the mirror block job iterates on the dirty bitmap
>>> whenever it feels like.
>>>
>>> There is no guarantee that the mirror block job has quiesced before
>>> migration handover takes place, right?
>>
>> Libvirt does that. Migration is started only once storage mirroring
>> is out of the bulk phase, and the handover looks like:
>>
>> 1) migration completes
>>
>> 2) because the source VM is stopped, the disk has quiesced on the source
>
> But the mirror block job might still be writing out dirty blocks.
>
>> 3) libvirt sends block-job-complete
>
> No, it sends block-job-cancel after the source QEMU's migration has
> completed. See the qemuMigrationCancelDriveMirror() call in
> src/qemu/qemu_migration.c:qemuMigrationRun().
>
>> 4) libvirt receives BLOCK_JOB_COMPLETED. The disk has now quiesced on
>> the destination as well.
>
> I don't see where this happens in the libvirt source code. Libvirt
> doesn't care about block job events for drive-mirror during migration.
>
> And that's why there could still be I/O going on (since
> block-job-cancel is asynchronous).
>
>> 5) the VM is started on the destination
>>
>> 6) the NBD server is stopped on the destination and the source VM is quit.
>>
>> It is actually a feature that storage migration is completed
>> asynchronously with respect to RAM migration. The problem is that
>> qcow2_invalidate_cache happens between (3) and (5), and it doesn't
>> like the concurrent I/O received by the NBD server.
>
> I agree that qcow2_invalidate_cache() (and any other invalidate cache
> implementations) need to allow concurrent I/O requests.
>
> Either I'm misreading the libvirt code or libvirt is not actually
> ensuring that the block job on the source has cancelled/completed
> before the guest is resumed on the destination. So I think there is
> still a bug, maybe Eric can verify this?
You may indeed be correct that libvirt is not waiting long enough for
the block job to be gone on the source before resuming on the
destination. I didn't write that particular code, so I'm cc'ing the
libvirt list, but I can try and take a look into it, since it's related
to code I've recently touched in getting libvirt to support active layer
block commit.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/2014 08:57 AM, Daniel P. Berrange wrote:
>>> + >> guest_ecn='off'/>
>>> +
>>
>> Are we stuck with names with underscores in our XML? I'm still not sure
>> if we've come up with the best naming for exposing all these knobs.
>
> I'm not really convinced having a 'guest_' prefix really buys
> us anything here, since there's no naming clash to avoid. Why
> don't we just kill the 'guest_' prefixes.
>
> NB, remember that precisely matching QEMU naming is a non-goal,
> we should be designing something that makes sense in general.
I agree; I'd be fine with:
with no need for a sub-structure. Yeah, we'll have to do some
glue logic to translate to qemu names, but that's what libvirt is for.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
---
docs/hooks.html.in | 11
src/qemu/qemu_driver.c | 69 +-
src/util/virhook.c | 3 ++-
src/util/virhook.h | 1 +
4 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/docs/hooks.html.in b/docs/hooks.html.in
index 07b9d49..265dbb7 100644
--- a/docs/hooks.html.in
+++ b/docs/hooks.html.in
@@ -177,6 +177,17 @@
script returns failure or the output XML is not valid, incoming
migration will be canceled. This hook may be used, e.g., to change
location of disk images for incoming domains.
+ Since 1.2.9, the qemu hook script is
+also called when restoring a saved image either via the API or
+automatically when restoring a managed save machine. It is called
+as: /etc/libvirt/hooks/qemu guest_name restore begin -
+with domain XML sent to standard input of the script. In this case,
+the script acts as a filter and is supposed to modify the domain
+XML and print it out on its standard output. Empty output is
+identical to copying the input XML without changing it. In case the
+script returns failure or the output XML is not valid, restore of the
+image will be aborted. This hook may be used, e.g., to change
+location of disk images for incoming domains.
Since 0.9.13, the qemu hook script
is also called when the libvirtd daemon restarts and reconnects
to previously running QEMU processes. If the script fails, the
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d82e93..2dd2e48 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5636,20 +5636,23 @@ qemuDomainRestoreFlags(virConnectPtr conn,
unsigned int flags)
{
virQEMUDriverPtr driver = conn->privateData;
+qemuDomainObjPrivatePtr priv = NULL;
virDomainDefPtr def = NULL;
-virDomainDefPtr newdef = NULL;
virDomainObjPtr vm = NULL;
+char *xml = NULL;
+char *xmlout = NULL;
int fd = -1;
int ret = -1;
virQEMUSaveHeader header;
virFileWrapperFdPtr wrapperFd = NULL;
+bool hook_taint = false;
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED, -1);
-fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL,
+fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml,
(flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
&wrapperFd, false, false);
if (fd < 0)
@@ -5658,12 +5661,29 @@ qemuDomainRestoreFlags(virConnectPtr conn,
if (virDomainRestoreFlagsEnsureACL(conn, def) < 0)
goto cleanup;
-if (dxml) {
-if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml)))
+if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
+int hookret;
+
+if ((hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name,
+ VIR_HOOK_QEMU_OP_RESTORE,
+ VIR_HOOK_SUBOP_BEGIN,
+ NULL,
+ dxml ? dxml : xml,
+ &xmlout)) < 0)
goto cleanup;
-virDomainDefFree(def);
-def = newdef;
+if (hookret == 0 && xmlout) {
+virDomainDefPtr tmp;
+
+VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout);
+
+if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlout)))
+goto cleanup;
+
+virDomainDefFree(def);
+def = tmp;
+hook_taint = true;
+}
}
if (!(vm = virDomainObjListAdd(driver->domains, def,
@@ -5679,6 +5699,11 @@ qemuDomainRestoreFlags(virConnectPtr conn,
else if (flags & VIR_DOMAIN_SAVE_PAUSED)
header.was_running = 0;
+if (hook_taint) {
+priv = vm->privateData;
+priv->hookRun = true;
+}
+
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
@@ -5697,6 +5722,8 @@ qemuDomainRestoreFlags(virConnectPtr conn,
cleanup:
virDomainDefFree(def);
VIR_FORCE_CLOSE(fd);
+VIR_FREE(xml);
+VIR_FREE(xmlout);
virFileWrapperFdFree(wrapperFd);
if (vm)
virObjectUnlock(vm);
@@ -5834,12 +5861,15 @@ qemuDomainObjRestore(virConnectPtr conn,
bool bypass_cache)
{
virDomainDefPtr def = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
int fd = -1;
int ret = -1;
+char *xml = NULL;
+char *xmlout = NULL;
virQEMUSaveHeader header;
virFileWrapperFdPtr wrapperFd = NULL;
-fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL,
+fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml,
bypass_cache, &wrapperFd, false, true);
if (fd < 0) {
if (fd == -3)
@@ -5847,6 +5877,29
Extract code used to check save image XMLs provided by users to separate
use.
---
src/qemu/qemu_driver.c | 100 -
1 file changed, 66 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 15ad64d..e41a08e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5311,6 +5311,68 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn,
return ret;
}
+
+/**
+ * qemuDomainSaveImageUpdateDef:
+ * @driver: qemu driver data
+ * @def: def of the domain from the save image
+ * @newxml: user provided replacement XML
+ *
+ * Returns the new domain definition in case @newxml is ABI compatible with the
+ * guest.
+ */
+static virDomainDefPtr
+qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
+ virDomainDefPtr def,
+ const char *newxml)
+{
+virDomainDefPtr ret = NULL;
+virDomainDefPtr newdef_migr = NULL;
+virDomainDefPtr newdef = NULL;
+virCapsPtr caps = NULL;
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (!(newdef = virDomainDefParseString(newxml, caps, driver->xmlopt,
+ QEMU_EXPECTED_VIRT_TYPES,
+ VIR_DOMAIN_XML_INACTIVE)))
+goto cleanup;
+
+if (!(newdef_migr = qemuDomainDefCopy(driver,
+ newdef,
+ VIR_DOMAIN_XML_MIGRATABLE)))
+goto cleanup;
+
+if (!virDomainDefCheckABIStability(def, newdef_migr)) {
+virResetLastError();
+
+/* Due to a bug in older version of external snapshot creation
+ * code, the XML saved in the save image was not a migratable
+ * XML. To ensure backwards compatibility with the change of the
+ * saved XML type, we need to check the ABI compatibility against
+ * the user provided XML if the check against the migratable XML
+ * fails. Snapshots created prior to v1.1.3 have this issue. */
+if (!virDomainDefCheckABIStability(def, newdef))
+goto cleanup;
+
+/* use the user provided XML */
+ret = newdef;
+newdef = NULL;
+} else {
+ret = newdef_migr;
+newdef_migr = NULL;
+}
+
+ cleanup:
+virObjectUnref(caps);
+virDomainDefFree(newdef);
+virDomainDefFree(newdef_migr);
+
+return ret;
+}
+
+
/* Return -1 on most failures after raising error, -2 if edit was specified
* but xmlin and state (-1 for no change, 0 for paused, 1 for running) do
* not represent any changes (no error raised), -3 if corrupt image was
@@ -5431,45 +5493,15 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
QEMU_EXPECTED_VIRT_TYPES,
VIR_DOMAIN_XML_INACTIVE)))
goto error;
-if (xmlin) {
-virDomainDefPtr def2 = NULL;
-virDomainDefPtr newdef = NULL;
-if (!(def2 = virDomainDefParseString(xmlin, caps, driver->xmlopt,
- QEMU_EXPECTED_VIRT_TYPES,
- VIR_DOMAIN_XML_INACTIVE)))
-goto error;
+if (xmlin) {
+virDomainDefPtr tmp;
-newdef = qemuDomainDefCopy(driver, def2, VIR_DOMAIN_XML_MIGRATABLE);
-if (!newdef) {
-virDomainDefFree(def2);
+if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin)))
goto error;
-}
-
-if (!virDomainDefCheckABIStability(def, newdef)) {
-virDomainDefFree(newdef);
-virResetLastError();
-
-/* Due to a bug in older version of external snapshot creation
- * code, the XML saved in the save image was not a migratable
- * XML. To ensure backwards compatibility with the change of the
- * saved XML type, we need to check the ABI compatibility against
- * the user provided XML if the check against the migratable XML
- * fails. Snapshots created prior to v1.1.3 have this issue. */
-if (!virDomainDefCheckABIStability(def, def2)) {
-virDomainDefFree(def2);
-goto error;
-}
-
-/* use the user provided XML */
-newdef = def2;
-def2 = NULL;
-} else {
-virDomainDefFree(def2);
-}
virDomainDefFree(def);
-def = newdef;
+def = tmp;
}
VIR_FREE(xml);
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
This series refactors (splits) the save image open function into separate chunks
and introduces a filter-hook that is called when restoring a save image.
Peter Krempa (5):
qemu: save image: Split out user provided XML checker
qemu: save image: Add possibility to return XML stored in the image
qemu: save image: Split out new definition check/update
qemu: save image: Split out checks done only when editing the save img
qemu: hook: Provide hook when restoring a domain save image
docs/hooks.html.in | 11 +++
src/qemu/qemu_driver.c | 261 ++---
src/util/virhook.c | 3 +-
src/util/virhook.h | 1 +
4 files changed, 195 insertions(+), 81 deletions(-)
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Sep 17, 2014 at 10:59:21AM -0400, John Ferlan wrote:
On 09/17/2014 10:00 AM, Martin Kletzander wrote:
On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote:
On 09/16/2014 05:16 AM, Martin Kletzander wrote:
This way it behaves more like the daemon itself does (acquiring a
pidfile, deleting the socket before binding, etc.).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604
Signed-off-by: Martin Kletzander
---
src/rpc/virnetsocket.c | 65 +++---
1 file changed, 57 insertions(+), 8 deletions(-)
The error path/retry logic needs a tweak... I added some inline thinking
since we don't have a virtual whiteboard to share on this!
Yes, it does. I didn't think it through from scratch, just adjusted
to your comments. This time I went through it few times. Just let me
confirm I understood what you meant everywhere.
You did :-)
<...snip...>
Wow, I just reached this part of the mail when I wrote it already :)
Funny how that happens.
My current diff to the previous version looks like this:
diff --git i/src/rpc/virnetsocket.c w/src/rpc/virnetsocket.c
index 7be1492..e0efb14 100644
--- i/src/rpc/virnetsocket.c
+++ w/src/rpc/virnetsocket.c
@@ -596,7 +596,6 @@ int virNetSocketNewConnectUNIX(const char *path,
goto error;
pidfd = virPidFileAcquirePath(pidpath, false, getpid());
-VIR_FREE(pidpath);
if (pidfd < 0) {
/*
* This can happen in a very rare case of two clients
spawning two
@@ -650,6 +649,7 @@ int virNetSocketNewConnectUNIX(const char *path,
* time without spawning the daemon.
*/
spawnDaemon = false;
+virPidFileDeletePath(pidpath);
VIR_FORCE_CLOSE(pidfd);
VIR_FORCE_CLOSE(passfd);
goto retry;
@@ -674,6 +674,7 @@ int virNetSocketNewConnectUNIX(const char *path,
* virCommandHook inside a virNetSocketForkDaemon().
*/
VIR_FORCE_CLOSE(pidfd);
+pidfd = -1;
VIR_FORCE_CLOSE() will do this for you
if (virNetSocketForkDaemon(binary, passfd) < 0)
goto error;
}
@@ -687,10 +688,12 @@ int virNetSocketNewConnectUNIX(const char *path,
if (!(*retsock = virNetSocketNew(&localAddr, &remoteAddr, true,
fd, -1, 0)))
goto error;
+VIR_FREE(pidpath);
+
return 0;
error:
-if (pidfd > 0)
+if (pidfd >= 0)
virPidFileDeletePath(pidpath);
VIR_FREE(pidpath);
VIR_FORCE_CLOSE(fd);
--
Is that fine or should I use that goto error; everywhere after
acquiring the pidfile or is it better for you to see it in another
version?
This is fine - I think things are now covered.
ACK
Thanks for you thorough review, I squashed it in, remove that one
unnecessary pidfd = -1 and pushed it.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/2014 10:00 AM, Martin Kletzander wrote:
> On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote:
>>
>>
>> On 09/16/2014 05:16 AM, Martin Kletzander wrote:
>>> This way it behaves more like the daemon itself does (acquiring a
>>> pidfile, deleting the socket before binding, etc.).
>>>
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604
>>>
>>> Signed-off-by: Martin Kletzander
>>> ---
>>> src/rpc/virnetsocket.c | 65
>>> +++---
>>> 1 file changed, 57 insertions(+), 8 deletions(-)
>>>
>>
>> The error path/retry logic needs a tweak... I added some inline thinking
>> since we don't have a virtual whiteboard to share on this!
>>
>
> Yes, it does. I didn't think it through from scratch, just adjusted
> to your comments. This time I went through it few times. Just let me
> confirm I understood what you meant everywhere.
>
You did :-)
<...snip...>
>
> Wow, I just reached this part of the mail when I wrote it already :)
Funny how that happens.
>
> My current diff to the previous version looks like this:
>
> diff --git i/src/rpc/virnetsocket.c w/src/rpc/virnetsocket.c
> index 7be1492..e0efb14 100644
> --- i/src/rpc/virnetsocket.c
> +++ w/src/rpc/virnetsocket.c
> @@ -596,7 +596,6 @@ int virNetSocketNewConnectUNIX(const char *path,
> goto error;
>
> pidfd = virPidFileAcquirePath(pidpath, false, getpid());
> -VIR_FREE(pidpath);
> if (pidfd < 0) {
> /*
> * This can happen in a very rare case of two clients
> spawning two
> @@ -650,6 +649,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> * time without spawning the daemon.
> */
> spawnDaemon = false;
> +virPidFileDeletePath(pidpath);
> VIR_FORCE_CLOSE(pidfd);
> VIR_FORCE_CLOSE(passfd);
> goto retry;
> @@ -674,6 +674,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> * virCommandHook inside a virNetSocketForkDaemon().
> */
> VIR_FORCE_CLOSE(pidfd);
> +pidfd = -1;
VIR_FORCE_CLOSE() will do this for you
> if (virNetSocketForkDaemon(binary, passfd) < 0)
> goto error;
> }
> @@ -687,10 +688,12 @@ int virNetSocketNewConnectUNIX(const char *path,
> if (!(*retsock = virNetSocketNew(&localAddr, &remoteAddr, true,
> fd, -1, 0)))
> goto error;
>
> +VIR_FREE(pidpath);
> +
> return 0;
>
> error:
> -if (pidfd > 0)
> +if (pidfd >= 0)
> virPidFileDeletePath(pidpath);
> VIR_FREE(pidpath);
> VIR_FORCE_CLOSE(fd);
> --
>
> Is that fine or should I use that goto error; everywhere after
> acquiring the pidfile or is it better for you to see it in another
> version?
>
>
This is fine - I think things are now covered.
ACK
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
> On 09/11/2014 05:43 AM, Ján Tomko wrote:
> > Add the following attributes:
> > csum, gso, guest_tso4, guest_tso6, guest_ecn
> > to the element of network interface
> > which control the virtio-net device properties
> > of the same names.
> > ---
> > docs/formatdomain.html.in | 27
> > docs/schemas/domaincommon.rng | 25 +++
> > src/conf/domain_conf.c | 81
> > ++
> > src/conf/domain_conf.h | 5 ++
> > .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +
> > tests/qemuxml2xmltest.c| 1 +
> > 6 files changed, 171 insertions(+)
> > create mode 100644
> > tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
> >
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index a2ea758..5b2758a 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
> >
> > > event_idx='off' queues='5'/>
> >
> > +
> > +
> > +
> > +
> > + > guest_ecn='off'/>
> > +
>
> Are we stuck with names with underscores in our XML? I'm still not sure
> if we've come up with the best naming for exposing all these knobs.
I'm not really convinced having a 'guest_' prefix really buys
us anything here, since there's no naming clash to avoid. Why
don't we just kill the 'guest_' prefixes.
NB, remember that precisely matching QEMU naming is a non-goal,
we should be designing something that makes sense in general.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Sep 17, 2014 at 16:53:02 +0200, Jiri Denemark wrote:
> This is a modified version of RDMA migration patches sent back in
> January by Michael R. Hines. See individual patches for (numerous)
> changes since v2.
>
> Jiri Denemark (3):
> qemu: Fix old tcp:host URIs more cleanly
> qemu: Prepare support for arbitrary migration protocol
> qemu: Add RDMA migration capabilities
>
> Michael R. Hines (3):
> qemu: Expose additional migration statistics
> qemu: RDMA migration support
> qemu: Memory pre-pinning support for RDMA migration
And this whole series should have been obviously marked as v3...
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
From: "Michael R. Hines"
This patch adds support for RDMA protocol in migration URIs.
USAGE: $ virsh migrate --live --migrateuri rdma://hostname domain
qemu+ssh://hostname/system
Since libvirt runs QEMU in a pretty restricted environment, several
files needs to be added to cgroup_device_acl (in qemu.conf) for QEMU to
be able to access the host's infiniband hardware. Full documenation of
the feature can be found on QEMU wiki:
http://wiki.qemu.org/Features/RDMALiveMigration
Signed-off-by: Michael R. Hines
Signed-off-by: Jiri Denemark
---
Notes:
The question is whether the IB devices should be added to
cgroup_device_acl by default or not...
Version 3:
- moved capabilities code into a separate patch
- got rid of migration URI hacks
- removed hacks that disabled IPv6 with RDMA
- moved refactoring into a dedicated patch
- documented IB devices which need to be added to cgroup acl in
qemu.conf
- forbid RDMA migrations unless memory hard limit is set until we
have a better plan for setting limits for mlock
- set QEMU's RLIMIT_MEMLOCK to memory hard_limit before starting
RDMA migration
- check if RDMA migration is supported by source QEMU before trying
to migrate
src/qemu/qemu.conf| 8
src/qemu/qemu_command.c | 8
src/qemu/qemu_migration.c | 39 +--
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 79bba36..92ca715 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -274,6 +274,14 @@
#"/dev/ptmx", "/dev/kvm", "/dev/kqemu",
#"/dev/rtc","/dev/hpet", "/dev/vfio/vfio"
#]
+#
+# RDMA migration requires the following extra files to be added to the list:
+# "/dev/infiniband/rdma_cm",
+# "/dev/infiniband/issm0",
+# "/dev/infiniband/issm1",
+# "/dev/infiniband/umad0",
+# "/dev/infiniband/umad1",
+# "/dev/infiniband/uverbs0"
# The default format for Qemu/KVM guest save images is raw; that is, the
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a892d99..fceed62 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9399,6 +9399,14 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
+} else if (STRPREFIX(migrateFrom, "rdma")) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("incoming RDMA migration is not supported "
+ "with this QEMU binary"));
+goto error;
+}
+virCommandAddArg(cmd, migrateFrom);
} else if (STREQ(migrateFrom, "stdio")) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
virCommandAddArgFormat(cmd, "fd:%d", migrateFd);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d0e2653..b59e94d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -56,6 +56,7 @@
#include "virhook.h"
#include "virstring.h"
#include "virtypedparam.h"
+#include "virprocess.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -2653,6 +2654,13 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
QEMU_MIGRATION_COOKIE_NBD)))
goto cleanup;
+if (STREQ(protocol, "rdma") && !vm->def->mem.hard_limit) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("cannot start RDMA migration with no memory hard "
+ "limit set"));
+goto cleanup;
+}
+
if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
goto cleanup;
qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PREPARE);
@@ -2696,6 +2704,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
goto stop;
+if (STREQ(protocol, "rdma") &&
+virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) {
+goto stop;
+}
+
if (mig->lockState) {
VIR_DEBUG("Received lockstate %s", mig->lockState);
VIR_FREE(priv->lockState);
@@ -2926,7 +2939,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
if (!(uri = qemuMigrationParseURI(uri_in, &well_formed_uri)))
goto cleanup;
-if (STRNEQ(uri->scheme, "tcp")) {
+if (STRNEQ(uri->scheme, "tcp") &&
+STRNEQ(uri->scheme, "rdma")) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
_("unsupported scheme %s in migration URI %s"),
uri->scheme, uri_in);
@@ -3545,6 +3559,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
switch (spec->destType) {
case MIGRATION_DEST_HOST:
+if (STREQ(spec->dest.host.protoc
The information in virDomainGetBlockInfo() is important for clients
that use qcow2 format on LVM block devices - by tracking the
allocation in relation to the physical size, management can tell
if a disk needs to be expanded before the guest (file system
contents) and/or qemu (copy-on-write differing more from a backing
file) runs out of space. Normally, only the active layer matters,
but during a block commit operation, the allocation of the backing
file ALSO grows, and management would like to track that growth.
Right now, virDomainGetBlockInfo() can only convey information
about the active layer of a disk, via a single API call per disk.
It can also be easily extended to support "vda[1]" notation that
we recently added for blockcommit and friends, to get similar
information about a backing element; but that still implies one
call per layer, which adds up to a lot of overhead.
This API addition will make it possible to grab this information
for ALL guest disks in a single call, by letting the user request
additional information about each disk in the backing chain to be
output as part of the domain XML. My ultimate goal is to have
this flag and virStorageVolGetXMLDesc() expose the same bits of
information about a given storage volume (there are slight
incompatiblities in the XML naming that we'll have to keep for
back-compat sake, but the idea is to get all the information out
there).
* include/libvirt/libvirt.h.in (virDomainXMLFlags): Add new flag.
* src/libvirt.c (virDomainGetXMLDesc): Document it.
(virDomainSaveImageGetXMLDesc, virDomainSnapshotGetXMLDesc): For
now, mention the flag won't help here.
* tools/virsh-domain.c (cmdDumpXML): Add new flag.
* tools/virsh.pod (dumpxml): Document it.
Signed-off-by: Eric Blake
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c| 15 +++
tools/virsh-domain.c | 6 ++
tools/virsh.pod | 6 --
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c2f9d26..40f4e0c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2082,6 +2082,7 @@ typedef enum {
VIR_DOMAIN_XML_INACTIVE = (1 << 1), /* dump inactive domain
information */
VIR_DOMAIN_XML_UPDATE_CPU = (1 << 2), /* update guest CPU requirements
according to host CPU */
VIR_DOMAIN_XML_MIGRATABLE = (1 << 3), /* dump XML suitable for migration
*/
+VIR_DOMAIN_XML_BLOCK_INFO = (1 << 4), /* include storage volume
information about each disk source */
} virDomainXMLFlags;
char * virDomainGetXMLDesc (virDomainPtr domain,
diff --git a/src/libvirt.c b/src/libvirt.c
index f7e5a37..1020cc5 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2806,8 +2806,9 @@ virDomainRestoreFlags(virConnectPtr conn, const char
*from, const char *dxml,
*
* No security-sensitive data will be included unless @flags contains
* VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections. For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * connections. For this API, @flags should not contain
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
*
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
* error. The caller must free() the returned value.
@@ -4354,6 +4355,11 @@ virDomainGetControlInfo(virDomainPtr domain,
* describing CPU capabilities is modified to match actual
* capabilities of the host.
*
+ * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each
+ * device will contain additional information such as capacity
+ * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(),
+ * and avoiding the need to call virDomainGetBlockInfo() separately.
+ *
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
* the caller must free() the returned value.
*/
@@ -18507,8 +18513,9 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
*
* No security-sensitive data will be included unless @flags contains
* VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections. For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * connections. For this API, @flags should not contain
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
*
* Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
* the caller must free() the returned value.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 435d045..8f97c48 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8838,6 +8838,10 @@ static const vshCmdOptDef opts_dumpxml[] = {
.type = VSH_OT_BOOL,
.help = N_("provide XML suitable for migrations")
},
+{.name = "block-info",
+ .type = VSH_OT_BOOL,
+
I'm still working on code to populate the latest numbers for
each disk of a domain, including getting numbers for offline
domains, but have confirmed that with these two patches alone
I'm able to see and numbers for block
volumes of live domains (thanks to how we populate backing
chain information). So while there are more patches to come,
I'd like to get review started on my proposed API addition.
Eric Blake (2):
dumpxml: add flag to virDomainGetXMLDesc
dumpxml: prepare to output block info
docs/schemas/domaincommon.rng | 22 ++
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c| 16 +++-
src/libvirt.c | 15 +++
src/util/virstoragefile.h | 3 ++-
tools/virsh-domain.c | 6 ++
tools/virsh.pod | 6 --
7 files changed, 61 insertions(+), 8 deletions(-)
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 17.09.2014 14:57, Cole Robinson wrote:
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
From: Cole Robinson
Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like
/path/to/ovmf
We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.
Signed-off-by: Michal Privoznik
ACK, thanks for cleaning it up. But please change authorship since the patch
is reasonably altered, I won't be offended :)
- Cole
Thanks. Fixed and pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote:
On 09/16/2014 05:16 AM, Martin Kletzander wrote:
This way it behaves more like the daemon itself does (acquiring a
pidfile, deleting the socket before binding, etc.).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604
Signed-off-by: Martin Kletzander
---
src/rpc/virnetsocket.c | 65 +++---
1 file changed, 57 insertions(+), 8 deletions(-)
The error path/retry logic needs a tweak... I added some inline thinking
since we don't have a virtual whiteboard to share on this!
Yes, it does. I didn't think it through from scratch, just adjusted
to your comments. This time I went through it few times. Just let me
confirm I understood what you meant everywhere.
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 80aeddf..7be1492 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -51,9 +51,11 @@
#include "virlog.h"
#include "virfile.h"
#include "virthread.h"
+#include "virpidfile.h"
#include "virprobe.h"
#include "virprocess.h"
#include "virstring.h"
+#include "dirname.h"
#include "passfd.h"
#if WITH_SSH2
@@ -541,7 +543,10 @@ int virNetSocketNewConnectUNIX(const char *path,
const char *binary,
virNetSocketPtr *retsock)
{
+char *binname = NULL;
+char *pidpath = NULL;
int fd, passfd = -1;
+int pidfd = -1;
virSocketAddr localAddr;
virSocketAddr remoteAddr;
@@ -580,16 +585,47 @@ int virNetSocketNewConnectUNIX(const char *path,
goto error;
}
+if (!(binname = last_component(binary)) || binname[0] == '\0') {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Cannot determine basename for binary '%s'"),
+ binary);
+goto error;
+}
+
+if (virPidFileConstructPath(false, NULL, binname, &pidpath) < 0)
+goto error;
Since the first param is false, we are guaranteed only that 'pidpath' is
the path to the virGetUserRuntimeDirectory() for "$binname.pid".
We are not sure if we created the path in virFileMakePathHelper() or
not. If we later then delete the file on the error path how does that
affect the daemon that wins the race? See the conundrum?
This is only about deleting the pidfile, right? Deleting it only when
it is acquired (pidfd >= 0) should fix this.
I'll try describing it here a little bit more:
virNetSocketNewConnectUNIX() is called with (spawnDaemon == true) only
if (privileged == false). virPidFileConstructPath() is called also
only when (spawnDaemon == true) and guarantees that the path for the
pidfile exists and is constructed the same way it is in daemon. The
path should not be deleted no matter whether we fail or not, those
directories should be kept there for later.
+
+pidfd = virPidFileAcquirePath(pidpath, false, getpid());
+VIR_FREE(pidpath);
Because you VIR_FREE() here, there is no way for the error: path to have
a non NULL pidpath... and delete the pidpath.
Using VIR_FREE(pidpath); in both error path and before return 0 (my
initial idea) should take care of this.
+if (pidfd < 0) {
Getting here means we were unable to get the pidfile lock and I don't
think we want to delete the pidpath... Since it's probably owned by some
other daemon
if (pidfd < 0) then it means it will not get deleted. By the way it
doesn't have to be deleted, closing should be enough to unlock the
file.
+/*
+ * This can happen in a very rare case of two clients spawning two
+ * daemons at the same time, and the error in the logs that gets
+ * reset here can be a clue to some future debugging.
+ */
+virResetLastError();
+spawnDaemon = false;
+goto retry;
+}
If we get here, we've written our pid into the pidfile and we have have
the lock... So that means we should own the file. Errors from here
should delete the file.
Right, there's nothing wrong.
+
if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) {
virReportSystemError(errno, "%s", _("Failed to create socket"));
goto error;
}
/*
- * We have to fork() here, because umask() is set
- * per-process, chmod() is racy and fchmod() has undefined
- * behaviour on sockets according to POSIX, so it doesn't
- * work outside Linux.
+ * We already even acquired the pidfile, so no one else should be using
+ * @path right now. So we're OK to unlink it and paying attention to
+ * the return value makes no real sense here. Only if it's not an
+ * abstract socket, of course.
+ */
+if (path[0] != '@')
+unlink(path);
+
+/*
+ * We have to fork(
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
> From: Cole Robinson
>
> Check to see if the UEFI binary mentioned in qemu.conf actually
> exists, and if so expose it in domcapabilities like
>
>
> /path/to/ovmf
>
>
> We introduce some generic domcaps infrastructure for handling
> a dynamic list of string values, it may be of use for future bits.
>
> Signed-off-by: Michal Privoznik
ACK, thanks for cleaning it up. But please change authorship since the patch
is reasonably altered, I won't be offended :)
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
> Up till now the virQEMUCapsFillDomainCaps() was type of void as
> there was no way for it to fail. This is, however, going to
> change in the next commit.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_capabilities.c | 25 -
> src/qemu/qemu_capabilities.h | 4 ++--
> src/qemu/qemu_driver.c | 3 ++-
> tests/domaincapstest.c | 19 ---
> 4 files changed, 32 insertions(+), 19 deletions(-)
ACK
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/17/14 14:15, Michal Privoznik wrote:
> Up till now the virQEMUCapsFillDomainCaps() was type of void as
> there was no way for it to fail. This is, however, going to
> change in the next commit.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_capabilities.c | 25 -
> src/qemu/qemu_capabilities.h | 4 ++--
> src/qemu/qemu_driver.c | 3 ++-
> tests/domaincapstest.c | 19 ---
> 4 files changed, 32 insertions(+), 19 deletions(-)
Seems reasonable.
Acked-by: Laszlo Ersek
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 17.09.2014 01:52, Cole Robinson wrote:
Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like
/path/to/ovmf
We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.
---
docs/formatdomaincaps.html.in | 6
docs/schemas/domaincaps.rng| 17 ++---
src/conf/domain_capabilities.c | 23
src/conf/domain_capabilities.h | 8 +
src/qemu/qemu_driver.c | 24 +
tests/domaincapsschemadata/domaincaps-full.xml | 1 +
tests/domaincapstest.c | 49 +-
7 files changed, 115 insertions(+), 13 deletions(-)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 34d746d..6959dfe 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -105,6 +105,7 @@
...
+ /usr/share/OVMF/OVMF_CODE.fdrompflash
@@ -122,6 +123,11 @@
For the loader element, the following can occur:
+ value
+ List of known loader paths. Currently this is only used
+ to advertise known locations of OVMF binaries for qemu. Binaries
+ will only be listed if they actually exist on disk.
+
type
Whether loader is a typical BIOS (rom) or
an UEFI binary (pflash). This refers to
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index ad8d966..dfdb9b9 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -46,6 +46,9 @@
+
+
+
I know it doesn't really matter, but I prefer attributes to be defined
before any child elements. So I'd move 'supported' a few lines up.
@@ -85,6 +88,14 @@
+
+
+
+
+
+
+
+
@@ -100,11 +111,7 @@
-
-
-
-
-
+
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 5a3c8e7..44e422a 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -46,6 +46,15 @@ static int virDomainCapsOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virDomainCaps)
+static void
+virDomainCapsValuesFree(virDomainCapsValuesPtr values)
+{
+size_t i;
+
+for (i = 0; i < values->nvalues; i++) {
+VIR_FREE(values->values[i]);
+}
+}
static void
virDomainCapsDispose(void *obj)
@@ -54,6 +63,8 @@ virDomainCapsDispose(void *obj)
VIR_FREE(caps->path);
VIR_FREE(caps->machine);
+
+virDomainCapsValuesFree(&caps->os.loader.values);
}
@@ -156,6 +167,17 @@ virDomainCapsEnumFormat(virBufferPtr buf,
return ret;
}
+static void
+virDomainCapsValuesFormat(virBufferPtr buf,
+ virDomainCapsValuesPtr values)
+{
+size_t i;
+
+for (i = 0; i < values->nvalues; i++) {
+virBufferAsprintf(buf, "%s\n", values->values[i]);
+}
+}
+
#define FORMAT_PROLOGUE(item) \
do {\
virBufferAsprintf(buf, "<" #item " supported='%s'%s\n", \
@@ -185,6 +207,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
{
FORMAT_PROLOGUE(loader);
+virDomainCapsValuesFormat(buf, &loader->values);
ENUM_PROCESS(loader, type, virDomainLoaderTypeToString);
ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString);
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 768646b..3d5aaa3 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -37,6 +37,13 @@ struct _virDomainCapsEnum {
unsigned int values; /* Bitmask of values supported in the corresponding
enum */
};
+typedef struct _virDomainCapsValues virDomainCapsValues;
+typedef virDomainCapsValues *virDomainCapsValuesPtr;
+struct _virDomainCapsValues {
+char **values; /* raw string values */
+size_t nvalues; /* number of strings */
+};
While this works, I'd rename this to virDomainCapsStringValues so that
it's clear what values do we have in mind. Moreover, if we ever
introduce other values (say list of integers) the environment is ready
and we don't have to do the renaming then.
+
typedef struct _virDomainCapsDevice virDomainCapsDevice;
typedef virDomainCapsDevice *virDomainCapsDevicePtr;
struct _virDomainCapsDevice {
@@ -47,6 +54,7 @@ typedef struct _virDomainCapsLoader virDomainCapsLoader;
typedef virDomainCapsLoader *virDomainCapsLoaderPtr;
struct _virDomainCapsLoader {
virDomainCapsDevice device;
+virDomainCapsValues values;
virDomain
On 09/17/2014 11:02 AM, Pradipta Kumar Banerjee wrote:
> On 09/15/2014 07:46 PM, Ján Tomko wrote:
>> On 09/13/2014 05:28 PM, Pradipta Kr. Banerjee wrote:
>>> qemu: Add support for multiple versions of 'pseries' machine type
>>>
>>> qemu for IBM Power processor architecture is adding functionality for
>>> supporting multiple 'pseries' machine type versions, each with different
>>> capabilities. This patch is for supporting the same
>>>
>>> Signed-off-by: Pradipta Kr. Banerjee
>>> ---
>>> src/qemu/qemu_command.c | 18 +-
>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> ACK.
>>
>> Would you like to add a qemuxml2argvtest for 'pseries-2.1' machine with these
>> devices?
> As of now there are no changes to qemu command line invocation based on
> different machine type. If required for this patch I'll go ahead and add the
> test cases for pseries-2.1 and pseries-2.2.
It is not required for the patch, I just thought a test would help to make
sure that we don't break the old machine types if we ever change something in
the code.
I've pushed the patch now.
> pseries-2.2 is the latest which
> aliases to pseries. Please do let me know your thoughts.
Oh, so it's the other way than on x86_64 where 'pc' is the alias for the
latest pc-i440fx machine (pc-i440fx-2.2 as of now).
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/15/2014 07:46 PM, Ján Tomko wrote:
> On 09/13/2014 05:28 PM, Pradipta Kr. Banerjee wrote:
>> qemu: Add support for multiple versions of 'pseries' machine type
>>
>> qemu for IBM Power processor architecture is adding functionality for
>> supporting multiple 'pseries' machine type versions, each with different
>> capabilities. This patch is for supporting the same
>>
>> Signed-off-by: Pradipta Kr. Banerjee
>> ---
>> src/qemu/qemu_command.c | 18 +-
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> ACK.
>
> Would you like to add a qemuxml2argvtest for 'pseries-2.1' machine with these
> devices?
As of now there are no changes to qemu command line invocation based on
different machine type. If required for this patch I'll go ahead and add the
test cases for pseries-2.1 and pseries-2.2. pseries-2.2 is the latest which
aliases to pseries. Please do let me know your thoughts.
>
> Jan
>
>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Regards,
Pradipta Kumar B(bpra...@in.ibm.com)
IBM Systems & Technology Labs,
India.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/16/2014 12:24 AM, John Ferlan wrote:
>
>
> On 09/11/2014 07:43 AM, Ján Tomko wrote:
>> Add the following attributes:
>> csum, gso, guest_tso4, guest_tso6, guest_ecn
>> to the element of network interface
>> which control the virtio-net device properties
>> of the same names.
>> ---
>> docs/formatdomain.html.in | 27
>> docs/schemas/domaincommon.rng | 25 +++
>> src/conf/domain_conf.c | 81
>> ++
>> src/conf/domain_conf.h | 5 ++
>> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +
>> tests/qemuxml2xmltest.c| 1 +
>> 6 files changed, 171 insertions(+)
>> create mode 100644
>> tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index a2ea758..5b2758a 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
>>
>>> event_idx='off' queues='5'/>
>>
>> +
>> +
>> +
>> +
>> + > guest_ecn='off'/>
>> +
>
> This doesn't require a driver name='' value?
>
AFAIK only queues require name='vhost'.
>>
>>...
>>
>> @@ -3949,6 +3955,27 @@ qemu-kvm -net nic,model=? /dev/null
>> processor, resulting in much higher throughput.
>> Since 1.0.6 (QEMU and KVM only)
>>
>> + csum
>> +
>> +The csum attribute with possible values on
>> +and off controls host-side support for packets with
>> +partial checksum values.
>> +Since 1.2.9 (QEMU only)
>> +
>> +In general you should leave this option alone, unless you
>> +are very certain you know what you are doing.
>> +
>> + segment offloading options
>> +
>> +The attributes gso, guest_tso4,
>> +guest_tso6, guest_ecn with possible
>> +values of on and off can be used
>> +to tune segment offloading.
>> +Since 1.2.9 (QEMU only)
>> +
>> +In general you should leave this option alone, unless you
>> +are very certain you know what you are doing.
>
> s/this option/these options/
>
> [oh - I'm having a flashback to something similar I had to do at my
> former employer with their virtualization switch software... these got
> enabled by some application and caused shall we say significant
> performance issues, especially for older drivers. That particular
> software was loaded/started after the virtualization network switch and
> thus reset what our code had done... Bugger to find as it embedded in
> some output from some network command that was executed rarely in our
> vswitch network daemon layer]
>
> Anyway, I understand it's desired to not say much about them, but is
> there any need to say what the defaults are? Furthermore, does one
> domain's setting affect other domains? I guess my curiosity is more is
> this a domain function or an interface (host) setting. We may want to
> indicate that here... Is the difference dependent upon the driver name?
>
This is a per (guest) interface setting, I'm not aware of it affecting other
interfaces or domains. Perhaps the 'leave this option alone' was too harsh.
I'll add the defaults before pushing/sending another version.
Jan
> I know from my previous experience it had a wider affect, but that code
> had a very different implementation. The vswitch was separate from the
> guest as a host process to which guests could configure ports. The
> vswitch software had the configuration magic to the lower level network
> driver(s) which is where the tso/cko, etc. were managed in the kernel.
> The equivalent of 'em0', 'eth0', etc - the physical NIC. A vswitch was
> "tied" to a particular physical NIC. So any changes to the pNIC cast a
> wide net, which is why "other" software setting TSO/CKO options on the
> same pNIC our vswitch used after our code disabled it caused all sorts
> of issues. (Just so you understand why I'm asking the question).
>
>> +
>>
>>
>> Overriding the target
>> element
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/16/2014 12:30 AM, Eric Blake wrote:
> On 09/11/2014 05:43 AM, Ján Tomko wrote:
>> Add the following attributes:
>> csum, gso, guest_tso4, guest_tso6, guest_ecn
>> to the element of network interface
>> which control the virtio-net device properties
>> of the same names.
>> ---
>> docs/formatdomain.html.in | 27
>> docs/schemas/domaincommon.rng | 25 +++
>> src/conf/domain_conf.c | 81
>> ++
>> src/conf/domain_conf.h | 5 ++
>> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +
>> tests/qemuxml2xmltest.c| 1 +
>> 6 files changed, 171 insertions(+)
>> create mode 100644
>> tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index a2ea758..5b2758a 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
>>
>>> event_idx='off' queues='5'/>
>>
>> +
>> +
>> +
>> +
>> + > guest_ecn='off'/>
>> +
>
> Are we stuck with names with underscores in our XML? I'm still not sure
> if we've come up with the best naming for exposing all these knobs.
I'd rather not mix underscores (event_idx) and other word separators in the
same element.
Alternatively, we could do something like:
to get rid of the multi-word attributes, but I like the underscores better
because they match QEMU arguments.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
