Re: [libvirt] Incompatible CPU after upgrading qemu
On 04/18/2017 12:35 AM, Jiri Denemark wrote: On Thu, Apr 13, 2017 at 16:11:26 -0600, Jim Fehlig wrote: Hi All, I've been doing some upgrade testing and noticed a problem starting some existing VMs after upgrading qemu from 2.6 to 2.9 on one of my AMD machines. Using libvirt 3.2.0 and qemu 2.6, I have no problems starting a VM with the following machine/CPU config hvm Opteron_G4 After upgrading qemu to 2.9 the VM fails to start with error: the CPU is incompatible with host CPU: Host CPU does not provide required features: svm I see that qemu commit 75d373ef disables SVM in KVM mode, but that change was made quite some time ago and pertains to all machine types > pc-i440fx-2.2. Perhaps the problem was revealed by Jiri's recent libvirt changes to detect host CPU model by asking qemu? My knee-jerk reaction is to remove 'svm' from Opteron CPUs in cpu_map.xml, but I'm far from the expert on the evolving CPU code in libvirt, so asking the experts here for some guidance. This issue should be fixed by "qemu: More CPU enhancements and fixes" series [1], specifically patch 9/9. See the commit message for more details. [1] https://www.redhat.com/archives/libvir-list/2017-April/msg00710.html Thanks. Although I haven't reviewed the series, I did test it and indeed it fixes the issue. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH go-xml] Support for filesystem devices
This commit adds filesystem device support. A new family of types
DomainFilesystem* are introduced and plumbed into the DomainDeviceList
struct.
Testing has also been included.
---
domain.go | 40
domain_test.go | 55 +++
2 files changed, 95 insertions(+)
diff --git a/domain.go b/domain.go
index 307c71c..cccd9a6 100644
--- a/domain.go
+++ b/domain.go
@@ -81,6 +81,45 @@ type DomainDisk struct {
Target *DomainDiskTarget `xml:"target"`
}
+type DomainFilesystemDriver struct {
+ Type string `xml:"type,attr"`
+ Name string `xml:"name,attr,omitempty"`
+ WRPolicy string `xml:"wrpolicy,attr,omitempty"`
+}
+
+type DomainFilesystemSource struct {
+ Dir string `xml:"dir,attr,omitempty"`
+ File string `xml:"file,attr,omitempty"`
+}
+
+type DomainFilesystemTarget struct {
+ Dir string `xml:"dir,attr"`
+}
+
+type DomainFilesystemReadOnly struct {
+}
+
+type DomainFilesystemSpaceHardLimit struct {
+ Value int`xml:",chardata"`
+ Unit string `xml:"unit,attr,omitempty"`
+}
+
+type DomainFilesystemSpaceSoftLimit struct {
+ Value int`xml:",chardata"`
+ Unit string `xml:"unit,attr,omitempty"`
+}
+
+type DomainFilesystem struct {
+ Type string `xml:"type,attr"`
+ AccessMode string `xml:"accessmode,attr"`
+ Driver *DomainFilesystemDriver `xml:"driver"`
+ Source *DomainFilesystemSource `xml:"source"`
+ Target *DomainFilesystemTarget `xml:"target"`
+ ReadOnly *DomainFilesystemReadOnly `xml:"readonly"`
+ SpaceHardLimit *DomainFilesystemSpaceHardLimit `xml:"space_hard_limit"`
+ SpaceSoftLimit *DomainFilesystemSpaceSoftLimit `xml:"space_soft_limit"`
+}
+
type DomainInterfaceMAC struct {
Address string `xml:"address,attr"`
}
@@ -212,6 +251,7 @@ type DomainVideo struct {
type DomainDeviceList struct {
Controllers []DomainController `xml:"controller"`
Disks []DomainDisk `xml:"disk"`
+ Filesystems []DomainFilesystem `xml:"filesystem"`
Interfaces []DomainInterface `xml:"interface"`
Serials []DomainChardev`xml:"serial"`
Consoles[]DomainChardev`xml:"console"`
diff --git a/domain_test.go b/domain_test.go
index e5347ea..06d585c 100644
--- a/domain_test.go
+++ b/domain_test.go
@@ -690,6 +690,61 @@ var domainTestData = []struct {
``,
},
},
+ {
+ Object: &Domain{
+ Type: "kvm",
+ Name: "test",
+ Devices: &DomainDeviceList{
+ Filesystems: []DomainFilesystem{
+ DomainFilesystem{
+ Type: "mount",
+ AccessMode: "mapped",
+ Driver: &DomainFilesystemDriver{
+ Type: "path",
+ WRPolicy: "immediate",
+ },
+ Source: &DomainFilesystemSource{
+ Dir: "/home/user/test",
+ },
+ Target: &DomainFilesystemTarget{
+ Dir: "user-test-mount",
+ },
+ },
+ DomainFilesystem{
+ Type: "file",
+ AccessMode: "passthrough",
+ Driver: &DomainFilesystemDriver{
+ Name: "loop",
+ Type: "raw",
+ },
+ Source: &DomainFilesystemSource{
+ File:
"/home/user/test.img",
+ },
+ Target: &DomainFilesystemTarget{
+ Dir:
"user-file-test-mount",
+ },
+ },
+ },
+ },
+ },
+ Expected: []string{
+ ``,
+ ` test`,
+ ` `,
+
Re: [libvirt] [PATCH 5/6] util: add virNumToStr
Ah, you're absolutely right, I completely forgot this function existed.
On Tue, Apr 18, 2017 at 5:18 PM Matthias Bolte <
matthias.bo...@googlemail.com> wrote:
> 2017-04-18 0:36 GMT+02:00 Sri Ramanujam :
> > Add virNumToStr(), which safely converts numbers into their string
> > representation.
> >
> > Functions added:
> > * virNumToStr_l
> > * virNumToStr_ul
> > ---
> > src/util/virstring.c | 34 ++
> > src/util/virstring.h | 8
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/src/util/virstring.c b/src/util/virstring.c
> > index 69abc26..f0d9e19 100644
> > --- a/src/util/virstring.c
> > +++ b/src/util/virstring.c
> > @@ -536,6 +536,40 @@ virStrToDouble(char const *s,
> > return 0;
> > }
> >
> > +/**
> > + * Converts signed number to string representation. The caller is
> responsible
> > + * for freeing the result.
> > + */
> > +int
> > +virNumToStr_l(long num, char **dst)
> > +{
> > +int sz;
> > +
> > +sz = snprintf(NULL, 0, "%ld", num);
> > +if (sz > 0 && VIR_ALLOC_N(*dst, sz + 1) < 0)
> > +return -1;
> > +
> > +snprintf(*dst, sz + 1, "%ld", num);
> > +return 0;
> > +}
> > +
> > +/**
> > + * Converts unsigned number to string representation. The caller is
> responsible
> > + * for freeing the result.
> > + */
> > +int
> > +virNumToStr_ul(unsigned long num, char **dst)
> > +{
> > +int sz;
> > +
> > +sz = snprintf(NULL, 0, "%lu", num);
> > +if (sz > 0 && VIR_ALLOC_N(*dst, sz + 1) < 0)
> > +return -1;
> > +
> > +snprintf(*dst, sz + 1, "%lu", num);
> > +return 0;
> > +}
> > +
>
> What's the gain of
>
> if (virNumToStr_ul(memory_mb, &memory_str) < 0)
>goto cleanup;
>
> over
>
> if (virAsprintf(&memory_str, "%lu", memory_mb) < 0)
>goto cleanup;
>
> ?
>
> I think those two new functions are not necessary at all.
>
> --
> Matthias Bolte
> http://photron.blogspot.com
>
--
*Sri Ramanujam*
Software Engineer
Datto, Inc.
www.datto.com
[image:
Twitter] [image: LinkedIn]
[image: pinterest]
[image: YouTube]
[image: Google Plus Page]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] hyperv: recognize array property as distinct type.
2017-04-18 16:56 GMT+02:00 Dawid Zamirski :
> When hyperv code generator for WMI classes identifies common
> properties, it needs to take into account array type as a distinct
> type, i.e string != string[]. This is the case where v1 of the
> Msvm_VirtualSystemSettingData has Notes property as string whereas v2
> uses Notes[], therefore they have to be treated as different fields and
> cannot be placed in the "common" struct.
> ---
>
> changes in v3:
> * add virBufferCheckError before calling virBufferContentAndReset.
>
> src/hyperv/hyperv_driver.c | 28 ++--
> src/hyperv/hyperv_wmi_generator.py | 2 +-
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 090ea24..8e5eeda 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -894,8 +894,32 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int
> flags)
> if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0)
> goto cleanup;
>
> -if (VIR_STRDUP(def->description,
> virtualSystemSettingData->data.common->Notes) < 0)
> -goto cleanup;
> +if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
> +if (VIR_STRDUP(def->description,
> + virtualSystemSettingData->data.v1->Notes) < 0)
> +goto cleanup;
> +} else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 &&
> + virtualSystemSettingData->data.v2->Notes.data != NULL) {
> +char **notes = (char **)
> virtualSystemSettingData->data.v2->Notes.data;
> +virBuffer buf = VIR_BUFFER_INITIALIZER;
> +size_t i = 0;
> +
> +/* in practice Notes has 1 element */
> +for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++)
> {
> +/* but if there's more than 1, separate by double new line */
> +if (virBufferUse(&buf) > 0)
> +virBufferAddLit(&buf, "\n\n");
> +
> +virBufferAdd(&buf, *notes, -1);
> +notes++;
> +}
> +
> +if (virBufferCheckError(&buf))
> +cleanup;
This will not compile, the goto is missing.
> +
> +def->description = virBufferContentAndReset(&buf);
> +}
> +
>
Extra whitespace.
ACK. I fixed both problems and pushed the result, thanks.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] util: add virNumToStr
2017-04-18 0:36 GMT+02:00 Sri Ramanujam :
> Add virNumToStr(), which safely converts numbers into their string
> representation.
>
> Functions added:
> * virNumToStr_l
> * virNumToStr_ul
> ---
> src/util/virstring.c | 34 ++
> src/util/virstring.h | 8
> 2 files changed, 42 insertions(+)
>
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 69abc26..f0d9e19 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -536,6 +536,40 @@ virStrToDouble(char const *s,
> return 0;
> }
>
> +/**
> + * Converts signed number to string representation. The caller is responsible
> + * for freeing the result.
> + */
> +int
> +virNumToStr_l(long num, char **dst)
> +{
> +int sz;
> +
> +sz = snprintf(NULL, 0, "%ld", num);
> +if (sz > 0 && VIR_ALLOC_N(*dst, sz + 1) < 0)
> +return -1;
> +
> +snprintf(*dst, sz + 1, "%ld", num);
> +return 0;
> +}
> +
> +/**
> + * Converts unsigned number to string representation. The caller is
> responsible
> + * for freeing the result.
> + */
> +int
> +virNumToStr_ul(unsigned long num, char **dst)
> +{
> +int sz;
> +
> +sz = snprintf(NULL, 0, "%lu", num);
> +if (sz > 0 && VIR_ALLOC_N(*dst, sz + 1) < 0)
> +return -1;
> +
> +snprintf(*dst, sz + 1, "%lu", num);
> +return 0;
> +}
> +
What's the gain of
if (virNumToStr_ul(memory_mb, &memory_str) < 0)
goto cleanup;
over
if (virAsprintf(&memory_str, "%lu", memory_mb) < 0)
goto cleanup;
?
I think those two new functions are not necessary at all.
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] xlconfigtest: add tests for 'nestedhvm' support
On Mon, 17 Apr 2017 14:25:13 -0600 Jim Fehlig wrote: > On 03/24/2017 03:02 PM, Wim Ten Have wrote: > > From: Wim ten Have > > > > Testing various configuration schemas targeting postive, negative > > and undefined nestedhvm under libvirt > > configuration. > > > > Mode "host-passthrough" generates nestedhvm=1 in/from xl format where > > > > Intel virtualization (VT-x): > > > > > > or > > > > AMD virtualization (AMD-V): > > > > > > disables virtualization mode under guest domains. > > > > Signed-off-by: Wim ten Have > > --- > > .../test-fullvirt-nestedhvm-disabled.cfg | 26 + > > .../test-fullvirt-nestedhvm-disabled.xml | 62 > > ++ > > .../test-fullvirt-nestedhvm-undefined.cfg | 25 + > > .../test-fullvirt-nestedhvm-undefined.xml | 58 > > > > The 'undefined' case is already tested by all the other tests that don't > contain > an explicit 'nestedhvm='. IMO it can be removed. O:-) ... will remove! Regards, - Wim. > > tests/xlconfigdata/test-fullvirt-nestedhvm.cfg | 26 + > > tests/xlconfigdata/test-fullvirt-nestedhvm.xml | 59 > > > > tests/xlconfigtest.c | 3 ++ > > 7 files changed, 259 insertions(+) > > create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg > > create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml > > create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.cfg > > create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.xml > > create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.cfg > > create mode 100644 tests/xlconfigdata/test-fullvirt-nestedhvm.xml > > > > diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg > > b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg > > new file mode 100644 > > index 000..d4b9f45 > > --- /dev/null > > +++ b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.cfg > > @@ -0,0 +1,26 @@ > > +name = "XenGuest2" > > +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" > > +maxmem = 579 > > +memory = 394 > > +vcpus = 1 > > +pae = 1 > > +acpi = 1 > > +apic = 1 > > +viridian = 0 > > +rtc_timeoffset = 0 > > +localtime = 0 > > +on_poweroff = "destroy" > > +on_reboot = "restart" > > +on_crash = "restart" > > +device_model = "/usr/lib/xen/bin/qemu-system-i386" > > +sdl = 0 > > +vnc = 1 > > +vncunused = 1 > > +vnclisten = "127.0.0.1" > > +vif = [ > > "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] > > +parallel = "none" > > +serial = "none" > > +builder = "hvm" > > +boot = "d" > > +nestedhvm = 0 > > +disk = [ > > "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", > > > > "format=qcow2,vdev=hdb,access=rw,backendtype=qdisk,target=/var/lib/libvirt/images/XenGuest2-home", > > > > "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" > > ] > > diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml > > b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml > > new file mode 100644 > > index 000..a5b9233 > > --- /dev/null > > +++ b/tests/xlconfigdata/test-fullvirt-nestedhvm-disabled.xml > > @@ -0,0 +1,62 @@ > > + > > + XenGuest2 > > + c7a5fdb2-cdaf-9455-926a-d65c16db1809 > > + 592896 > > + 403456 > > + 1 > > + > > +hvm > > +/usr/lib/xen/boot/hvmloader > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + destroy > > + restart > > + restart > > + > > +/usr/lib/xen/bin/qemu-system-i386 > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > diff --git a/tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.cfg > > b/tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.cfg > > new file mode 100644 > > index 000..4fe76b2 > > --- /dev/null > > +++ b/tests/xlconfigdata/test-fullvirt-nestedhvm-undefined.cfg > > @@ -0,0 +1,25 @@ > > +name = "XenGuest2" > > +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" > > +maxmem = 579 > > +memory = 394 > > +vcpus = 1 > > +pae = 1 > > +acpi = 1 > > +apic = 1 > > +viridian = 0 > > +rtc_timeoffset = 0 > > +localtime = 0 > > +on_poweroff = "destroy" > > +on_reboot = "restart" > > +on_crash = "restart" > > +device_model = "/usr/lib/xen/bin/qemu-system-i386" > > +sdl = 0 > > +vnc = 1 > > +vncunused = 1 > > +vnclisten = "127.0.0.1" > > +vif = [ > > "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] > > +parallel = "none" > > +serial = "none" > > +builder = "hvm" > > +boot = "d" > > +disk = [ > > "format=raw,vdev=hd
Re: [libvirt] [PATCH v2 2/3] xenconfig: add conversions for xen-xl
On Mon, 17 Apr 2017 14:22:20 -0600
Jim Fehlig wrote:
> On 03/24/2017 03:02 PM, Wim Ten Have wrote:
> > From: Wim ten Have
> >
> > Per xen-xl conversions from and to native under host-passthrough
> > mode we take care for Xen (nestedhvm = mode) applied and inherited
> > settings generating or processing correct feature policy:
> >
> > [On Intel (VT-x) architectures]
> >
> >
> > or
> >
> > [On AMD (AMD-V) architectures]
> >
> >
> > It will then generate (or parse) for nestedhvm=1 in/from xl format.
> >
> > Signed-off-by: Joao Martins
> > Signed-off-by: Wim ten Have
> > ---
> > src/xenconfig/xen_xl.c | 67
> > ++
> > 1 file changed, 67 insertions(+)
> >
> > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
> > index 74f68b3..d3797b8 100644
> > --- a/src/xenconfig/xen_xl.c
> > +++ b/src/xenconfig/xen_xl.c
> > @@ -106,6 +106,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def,
> > virCapsPtr caps)
> > if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
> > const char *bios;
> > const char *boot;
> > +int val = 0;
> >
> > if (xenConfigGetString(conf, "bios", &bios, NULL) < 0)
> > return -1;
> > @@ -164,6 +165,47 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def,
> > virCapsPtr caps)
> > }
> > def->os.nBootDevs++;
> > }
> > +
> > +if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
> > +return -1;
> > +
> > +if (val != -1) {
> > +virCPUDefPtr cpu = NULL;
> > +
> > +if (VIR_ALLOC(cpu) < 0)
> > +return -1;
> > +
> > +if (val == 0) {
> > +if (VIR_ALLOC_N(cpu->features, 2) < 0)
> > +goto cleanup;
> > +
> > +/*
> > + * Below is pointless in real world but for purpose
> > + * of testing let's check features depth holding at
> > + * least multiple elements and also check if both
> > + * vmx|svm are understood.
> > + */
> > +cpu->features[0].policy = VIR_CPU_FEATURE_DISABLE;
> > +if (VIR_STRDUP(cpu->features[0].name, "vmx") < 0)
> > +goto cleanup;
> > +cpu->features[1].policy = VIR_CPU_FEATURE_DISABLE;
> > +if (VIR_STRDUP(cpu->features[1].name, "svm") < 0)
> > +goto cleanup;
>
> Since caps is passed to this function, can it be used to determine whether to
> disable vmx or svm?
Yes :-) ... thanks for enlightening me here as that is actually the correct
approach if non-test domain (conversion) actions are made under libvirtd.
There's one minor question here per me. To process caps for vmx|svm I'll
bring in virCPUCheckFeature() which at its turn requires me to include
"cpu/cpu.h"
for its prototype. Unfortunate cfg.mk does not anticipate and raises a
syntax-check caveat.
prohibit_cross_inclusion
src/xenconfig/xen_xl.c:51:#include "cpu/cpu.h"
maint.mk: unsafe cross-directory include
cfg.mk:773: recipe for target 'sc_prohibit_cross_inclusion' failed
make: *** [sc_prohibit_cross_inclusion] Error 1
That is ... unless I apply below change to cfg.mk.
- xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen)";; \
+ xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
Is that lethal to do? I tried to reason and fail to see why not, ie. i
am a bit clueless for its specific reason ... but also new to whole arena.
> > +
> > +cpu->nfeatures = cpu->nfeatures_max = 2;
> > +}
> > +cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
> > +cpu->type = VIR_CPU_TYPE_GUEST;
> > +def->cpu = cpu;
> > +cpu = NULL;
> > + cleanup:
> > +if (cpu) {
> > +VIR_FREE(cpu->features);
> > +VIR_FREE(cpu);
> > +return -1;
> > +}
>
> I'm not fond of the cleanup label in the middle of this function. If we can
> disable only one of vmx or svm, then there will be one less strdup and one
> less
> cleanup spot. Cleanup can then occur at the point of error.
Correct and given former change this will now nicely fold down under its
eventual failing VIR_STRDUP().
> > +}
> > +
> > } else {
> > if (xenConfigCopyStringOpt(conf, "bootloader",
> > &def->os.bootloader) < 0)
> > return -1;
> > @@ -897,6 +939,31 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
> > if (xenConfigSetString(conf, "boot", boot) < 0)
> > return -1;
> >
> > +if (def->cpu &&
> > +def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
> > +bool hasHwVirt = true;
> > +
> > +if (def->cpu->nfeatures) {
> > +for (i = 0; i < def->cpu->nfeatures; i++) {
> > +
> > +
Re: [libvirt] [PATCH v2 1/3] libxl: set nestedhvm for mode host-passthrough
On Mon, 17 Apr 2017 12:04:53 -0600
Jim Fehlig wrote:
> On 03/24/2017 03:02 PM, Wim Ten Have wrote:
> > From: Wim ten Have
> >
> > Xen feature nestedhvm is the option on Xen 4.4+ which enables
> > nested virtualization when mode host-passthrough is applied.
> >
> > Virtualization on target domain can be disabled by specifying
> > such under feature policy rule on target name;
> >
> > [On Intel (VT-x) architecture]
> >
> >
> > or:
> >
> > [On AMD (AMD-V) architecture]
> >
>
> I think we should also give an example of enabling nested HVM. E.g.
>
>
Agree, will add.
> > Signed-off-by: Joao Martins
> > Signed-off-by: Wim ten Have
> > ---
> > src/libxl/libxl_conf.c | 47
> > ++-
> > src/libxl/libxl_conf.h | 2 +-
> > src/libxl/libxl_domain.c | 2 +-
> > 3 files changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> > index f5b788b..ede7c8a 100644
> > --- a/src/libxl/libxl_conf.c
> > +++ b/src/libxl/libxl_conf.c
> > @@ -47,6 +47,7 @@
> > #include "libxl_utils.h"
> > #include "virstoragefile.h"
> > #include "secret_util.h"
> > +#include "cpu/cpu.h"
> >
> >
> > #define VIR_FROM_THIS VIR_FROM_LIBXL
> > @@ -292,7 +293,7 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
> >
> > static int
> > libxlMakeDomBuildInfo(virDomainDefPtr def,
> > - libxl_ctx *ctx,
> > + libxlDriverConfigPtr cfg,
> >libxl_domain_config *d_config)
> > {
> > libxl_domain_build_info *b_info = &d_config->b_info;
> > @@ -308,7 +309,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> > libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
> >
> > b_info->max_vcpus = virDomainDefGetVcpusMax(def);
> > -if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus,
> > b_info->max_vcpus))
> > +if (libxl_cpu_bitmap_alloc(cfg->ctx, &b_info->avail_vcpus,
> > b_info->max_vcpus))
> > return -1;
> > libxl_bitmap_set_none(&b_info->avail_vcpus);
> > for (i = 0; i < virDomainDefGetVcpus(def); i++)
> > @@ -374,6 +375,42 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> >def->features[VIR_DOMAIN_FEATURE_ACPI] ==
> >VIR_TRISTATE_SWITCH_ON);
> >
> > +if (cfg && def->cpu &&
> > + def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
> > +bool hasHwVirt = false;
> > +bool svm = false, vmx = false;
> > +virCapsPtr caps = cfg->caps;
> > +
> > +if (caps && ARCH_IS_X86(def->os.arch)) {
> > +virCPUDefPtr cpuDef = caps->host.cpu;
>
> I don't see much value in having this local variable.
Indeed redundant (will remove).
> > +
> > +vmx = virCPUCheckFeature(
> > +cfg->caps->host.arch, cpuDef, "vmx");
> > +svm = virCPUCheckFeature(
> > +cfg->caps->host.arch, cpuDef, "svm");
> > +hasHwVirt = (vmx | svm);
> > +}
>
>
> And you already have a local 'caps' for cfg->caps. So this could be shortened
> a
> bit to
>
> vmx = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "vmx");
> svm = virCPUCheckFeature(caps->host.arch, caps->host.cpu, "svm");
> hasHwVirt = vmx | svm;
Agree.
> > +
> > +if (def->cpu->nfeatures) {
> > +for (i = 0; i < def->cpu->nfeatures; i++) {
> > +
> > +switch (def->cpu->features[i].policy) {
> > +
> > +case VIR_CPU_FEATURE_DISABLE:
> > +case VIR_CPU_FEATURE_FORBID:
> > +if ((vmx && STREQ(def->cpu->features[i].name,
> > "vmx")) ||
> > +(svm && STREQ(def->cpu->features[i].name,
> > "svm")))
> > +hasHwVirt = false;
> > +break;
> > +
> > +default:
> > +break;
>
> Generally libvirt prefers to explicitly list all enum values in switch
> statements, e.g.
>
> case VIR_CPU_FEATURE_FORCE:
> case VIR_CPU_FEATURE_REQUIRE:
> case VIR_CPU_FEATURE_OPTIONAL:
> case VIR_CPU_FEATURE_LAST:
> break;
Ah, i was not aware but see it around and it makes sense. I'll apply.
> > +}
> > +}
> > +}
> > +libxl_defbool_set(&b_info->u.hvm.nested_hvm, hasHwVirt);
> > +}
> > +
> > if (def->nsounds > 0) {
> > /*
> > * Use first sound device. man xl.cfg(5) describes soundhw as
> > @@ -2087,15 +2124,15 @@ int
> > libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
> > virDomainDefPtr def,
> > const char *channelDir
Re: [libvirt] [PATCH] xenconfig: avoid double free on OOM testing
Peter Krempa wrote: > On Thu, Apr 13, 2017 at 11:07:41 -0600, Jim Fehlig wrote: >> Fix xlconfig channel tests when OOM testing is enabled. >> >> TEST: xlconfigtest >> 32) Xen XL-2-XML Format channel-unix ... OK >> Test OOM for nalloc=55 >> *** Error in >> `/home/jfehlig/virt/upstream/libvirt/tests/.libs/xlconfigtest': double free >> or corruption (fasttop): 0x00679550 *** >> ... >> (gdb) bt >> #0 0x736875af in raise () from /lib64/libc.so.6 >> #1 0x736889aa in abort () from /lib64/libc.so.6 >> #2 0x736c5150 in __libc_message () from /lib64/libc.so.6 >> #3 0x736cb4f6 in malloc_printerr () from /lib64/libc.so.6 >> #4 0x736cbcee in _int_free () from /lib64/libc.so.6 >> #5 0x7782babf in virFree (ptrptr=0x7fffdca8) at >> util/viralloc.c:582 >> #6 0x0042f2f3 in xenParseXLChannel (conf=0x677350, def=0x6815b0) at >> xenconfig/xen_xl.c:788 >> #7 0x0042f44e in xenParseXL (conf=0x677350, caps=0x6832b0, >> xmlopt=0x67f6e0) at xenconfig/xen_xl.c:828 >> #8 0x004105a3 in testCompareFormatXML ( >> xlcfg=0x6811e0 >> "/home/jfehlig/virt/upstream/libvirt/tests/xlconfigdata/test-channel-unix.cfg", >> xml=0x681110 >> "/home/jfehlig/virt/upstream/libvirt/tests/xlconfigdata/test-channel-unix.xml", >> replaceVars=false) >> at xlconfigtest.c:152 >> >> When a channel is successfully parsed and its path and name fields >> assigned from local variables, set the local variables to NULL to >> prevent a double free on error. >> >> Signed-off-by: Jim Fehlig >> --- >> src/xenconfig/xen_xl.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) > > ACK Thanks, pushed now. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] VHPC at ISC extension - Papers due May 2
CALL FOR PAPERS 12th Workshop on Virtualization in High-Performance Cloud Computing (VHPC '17) held in conjunction with the International Supercomputing Conference - High Performance, June 18-22, 2017, Frankfurt, Germany. (Springer LNCS Proceedings) Date: June 22, 2017 Workshop URL: http://vhpc.org Paper Submission Deadline: May 2, 2017 (extended), Springer LNCS, rolling abstract submission Abstract/Paper Submission Link: https://edas.info/newPaper.php?c=23179 Keynotes: Satoshi Matsuoka, Professor of Computer Science, Tokyo Institute of Technology and John Goodacre, Professor in Computer Architectures University of Manchester, Director of Technology and Systems ARM Ltd. Research Group and Chief Scientific Officer Kaleao Ltd. Call for Papers Virtualization technologies constitute a key enabling factor for flexible resource management in modern data centers, and particularly in cloud environments. Cloud providers need to manage complex infrastructures in a seamless fashion to support the highly dynamic and heterogeneous workloads and hosted applications customers deploy. Similarly, HPC environments have been increasingly adopting techniques that enable flexible management of vast computing and networking resources, close to marginal provisioning cost, which is unprecedented in the history of scientific and commercial computing. Various virtualization technologies contribute to the overall picture in different ways: machine virtualization, with its capability to enable consolidation of multiple underutilized servers with heterogeneous software and operating systems (OSes), and its capability to live-migrate a fully operating virtual machine (VM) with a very short downtime, enables novel and dynamic ways to manage physical servers; OS-level virtualization (i.e., containerization), with its capability to isolate multiple user-space environments and to allow for their coexistence within the same OS kernel, promises to provide many of the advantages of machine virtualization with high levels of responsiveness and performance; I/O Virtualization allows physical NICs/HBAs to take traffic from multiple VMs or containers; network virtualization, with its capability to create logical network overlays that are independent of the underlying physical topology and IP addressing, provides the fundamental ground on top of which evolved network services can be realized with an unprecedented level of dynamicity and flexibility; the increasingly adopted paradigm of Software-Defined Networking (SDN) promises to extend this flexibility to the control and data planes of network paths. Publication Accepted papers will be published in a Springer LNCS proceedings volume. Topics of Interest The VHPC program committee solicits original, high-quality submissions related to virtualization across the entire software stack with a special focus on the intersection of HPC and the cloud. Major Topics - Virtualization in supercomputing environments, HPC clusters, HPC in the cloud and grids - OS-level virtualization and containers (Docker, rkt, Singularity, Shifter, i.a.) - Lightweight/specialized operating systems, unikernels - Optimizations of virtual machine monitor platforms and hypervisors - Hypervisor support for heterogenous resources (GPUs, co-processors, FPGAs, etc.) - Virtualization support for emerging memory technologies - Virtualization in enterprise HPC and microvisors - Software defined networks and network virtualization - Management, deployment of virtualized environments and orchestration (Kubernetes i.a.), - Workflow-pipeline container-based composability - Performance measurement, modelling and monitoring of virtualized/cloud workloads - Virtualization in data intensive computing and Big Data processing - HPC convergence - Adaptation of HPC technologies in the cloud (high performance networks, RDMA, etc.) - ARM-based hypervisors, ARM virtualization extensions - I/O virtualization and cloud based storage systems - GPU, FPGA and many-core accelerator virtualization - Job scheduling/control/policy and container placement in virtualized environments - Cloud reliability, fault-tolerance and high-availability - QoS and SLA in virtualized environments - IaaS platforms, cloud frameworks and APIs - Large-scale virtualization in domains such as finance and government - Energy-efficient and power-aware virtualization - Container security - Configuration management tools for containers (including CFEngine, Puppet, i.a.) - Emerging topics including multi-kernel approaches and,NUMA in hypervisors The Workshop on Virtualization in High-Performance Cloud Computing (VHPC) aims to bring together researchers and industrial practitioners facing the challenges posed by virtualization in order to foster discussion, collaboration, mutual excha
[libvirt] [PATCH 1/1] Enable Travis CI build status icon
Using GitHub libvirt site, it is possible to show Travis's fancy icon of the current build status. It highlights the QA process. --- README.md | 12 1 file changed, 12 insertions(+) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 000..a609286 --- /dev/null +++ b/README.md @@ -0,0 +1,12 @@ +## *LibVirt: the virtualization API* [](https://travis-ci.org/libvirt/libvirt) + + Libvirt is a C toolkit to interact with the virtualization capabilities +of recent versions of Linux (and other OSes). It is free software +available under the GNU Lesser General Public License. Virtualization of +the Linux Operating System means the ability to run multiple instances of +Operating Systems concurrently on a single hardware system where the basic +resources are driven by a Linux instance. The library aim at providing +long term stable C API initially for the Xen paravirtualization but +should be able to integrate other virtualization mechanisms if needed. + +Daniel Veillard -- 2.11.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Enable Travis build status icon
Follow up of the recent 'work in progress' announced in "Enable CI build testing with Travis" mailing message. Claudio André (1): Enable Travis CI build status icon README.md | 12 1 file changed, 12 insertions(+) create mode 100644 README.md -- 2.11.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix error reporting when poll returns POLLHUP/POLLERR
In the RPC client event loop code, if poll() returns only a POLLHUP
or POLLERR status, then we end up reporting a bogus error message:
error: failed to connect to the hypervisor
error: An error occurred, but the cause is unknown
We do actually report an error, but we virNetClientMarkClose method
has already captured the error status before we report it, so the
real error gets thrown away. The key fix is to report the error
before calling virNetClientMarkClose(). In changing this, we also
split out reporting of POLLHUP vs POLLERR to make any future bugs
easier to diagnose.
Signed-off-by: Daniel P. Berrange
---
src/rpc/virnetclient.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 5174614..c959747 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1743,10 +1743,16 @@ static int virNetClientIOEventLoop(virNetClientPtr
client,
if (error)
goto error;
-if (fds[0].revents & (POLLHUP | POLLERR)) {
+if (fds[0].revents & POLLHUP) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("received hangup event on socket"));
virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_EOF);
+goto error;
+}
+if (fds[0].revents & POLLERR) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("received hangup / error event on socket"));
+ _("received error event on socket"));
+virNetClientMarkClose(client, VIR_CONNECT_CLOSE_REASON_ERROR);
goto error;
}
}
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] autogen.sh: Rewrite and modularize
On Tue, Apr 18, 2017 at 04:31:16PM +0200, Andrea Bolognani wrote: > This work was motivated by Dan's intention to introduce the > keycodemapdb module into libvirt[1]. As it is, we have a few > useful submodule-related features in our build system, but > they are limited in that they assume there will be a single > submodule, and it will be gnulib. What is it that makes out submodule handling so complicated. eg in gtk-vnc and spice-gtk, we have a trivial line in autogen.sh 'git submodule update --init --recursive' that takes care of it. Is all the complexity we see caused by the need to detect when to re-run bootstrap for gnulib ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] autogen.sh: Modularize
On Tue, 2017-04-18 at 16:31 +0200, Andrea Bolognani wrote: > This sets up a generic framework for dealing with git > submodules, removing the assumption that gnulib is going to > be the only git submodule libvirt uses and making it > relatively easy for more to be added without sacrificing > the convenience we've gotten used to, eg. automatic updates. This should be squashed in so that we ship all the files required to regenerate the build system: diff --git a/Makefile.am b/Makefile.am index 333ec5a..df0c851 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,6 +38,7 @@ EXTRA_DIST = \ libvirt-admin.pc.in \ Makefile.nonreentrant \ autogen.sh \ + .submodules/gnulib.functions \ cfg.mk \ run.in \ AUTHORS.in -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Add mdev reporting capability to the nodedev driver
On Tue, 18 Apr 2017 09:52:38 +0100 "Daniel P. Berrange" wrote: > On Tue, Apr 18, 2017 at 10:49:40AM +0200, Martin Polednik wrote: > > On 12/04/17 16:19 -0600, Alex Williamson wrote: > > > On Wed, 5 Apr 2017 09:21:17 +0100 > > > "Daniel P. Berrange" wrote: > > > > > > > On Wed, Apr 05, 2017 at 08:12:31AM +0200, Erik Skultety wrote: > > > > > On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote: > > > > > > On Wed, Mar 29, 2017 at 02:51:10PM +0200, Erik Skultety wrote: > > > > > > > This series enables the node device driver to report information > > > > > > > about the > > > > > > > existing mediated devices on the host. There is no device > > > > > > > creation involved > > > > > > > yet. The information reported by the node device driver is split > > > > > > > into two > > > > > > > parts, one that is reported within the physical parent's > > > > > > > capabilities > > > > > > > (the generic stuff that comes from the mdev types' sysfs > > > > > > > attributes, note the > > > > > > > 'description' attribute which is verbatim - raw,unstructured > > > > > > > string) and the > > > > > > > other that is reported within the mdev child device and merely > > > > > > > contains the > > > > > > > mdev type id, which the device was instantiated from, and the > > > > > > > iommu group > > > > > > > number. > > > > > > > > > > > > > > Basically, the format of the XML I went for is as follows: > > > > > > > > > > > > > > PCI parent: > > > > > > > > > > > > > > pci__06_00_0 > > > > > > > /sys/devices/.../:06:00.0 > > > > > > > pci__05_08_0 > > > > > > > ... > > > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > > > > GRID M60-0B > > > > > > > num_heads=2, frl_config=45, > > > > > > > framebuffer=512M, max_resolution=2560x1600, > > > > > > > max_instance=16 > > > > > > > > > > > > This 'description' field is pretty horrific. > > > > > > > > > > > > We were quite clear that we were not going to expose arbitrary > > > > > > attributes > > > > > > in the XML without modelling them explicitly as XML elements. Using > > > > > > the > > > > > > description field in this way is just doing arbitrary attribute > > > > > > passthrough > > > > > > via the backdoor - it is clear that applications are doing to end > > > > > > up parsing > > > > > > this 'description' data and will them complain if we later change > > > > > > it. > > > > > > > > > > > > > > > > I remember us stating that, but this is the case with NVIDIA (who at > > > > > least > > > > > reports some useful information in it) and Intel - what if other > > > > > vendor comes > > > > > and creates a valid, human readable description of a device without > > > > > specifying > > > > > any attributes like in the case above? Just because some vendors > > > > > "abused" the > > > > > attribute doesn't mean we should stop reporting it completely. IMHO > > > > > the name > > > > > "description" should mean that it's something raw, possibly > > > > > unstructured, and > > > > > thus the applications should treat it that way. Now, this is my bad > > > > > and I need > > > > > to revisit the last patch with documentation and add a paragraph for > > > > > the > > > > > element as an optional element with raw data. > > > > > > > > > > Until all the attributes are properly unified/standardized under > > > > > sysfs ABI and > > > > > respected by vendors, I think we should report everything we're able > > > > > to gather > > > > > about a device, description included. If properly documented, nobody > > > > > can > > > > > complain about us breaking anything, since how do you guarantee > > > > > format-less raw > > > > > string anyway? After all, applications will end up parsing it anyway, > > > > > be it from > > > > > our XML or not. > > > > > > > > I understand your point, but I'm still not in favour of exposing this > > > > info > > > > because it is a clear cut attempt to do arbitrary attribute passthrough > > > > to > > > > libvirt. > > > > > > > > All the attributes show there can be determined by a simple lookup > > > > based on > > > > the name field "GRID M60-0B". So if apps want to know the number of > > > > heads, > > > > framebuffer size, etc, etc I think they should just maintain a lookup > > > > map > > > > based on name in their own code, until we explicitly model this stuff in > > > > the XML. > > > > > > > > Once we model the attributes in the XML, this description adds no useful > > > > info that we wouldn't already be reported, and before we model it in the > > > > XML, this is just clearly an abuse of our design statement that we are > > > > not > > > > doing generic attribute passthrough. > > > > > > I told Erik I'd jump in here, but I think I might agree with Dan. On > > > the kernel side, the description attribute was an attempt to pull > > > ourselves out of a rat hole of trying to figure out how to classify > > > d
Re: [libvirt] Trouble with virsh on Windows
On 4/18/2017 11:09 AM, Daniel P. Berrange wrote: > FYI, I've tracked this down to a likely bug in gnulib. All versions of > libvirt from 1.2.14 onwards are affected & so will fail to connect. > > If you don't need the newer APIs, you could try using libvirt 1.2.13 for > your Windows builds - I've tested that at least successfully connects. > > Meanwhile we'll aim to get this fixed in the next release (v3.3.0) > > Regards, > Daniel I'll take a look at 1.2.13 and see what is different. Worst case we will have some reduced functionality in our Windows client until the next release. Thanks for looking into this Daniel. It is much appreciated. -Charles signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Trouble with virsh on Windows
On Tue, Apr 18, 2017 at 01:55:17PM +0100, Daniel P. Berrange wrote: > On Tue, Apr 18, 2017 at 01:41:27PM +0100, Daniel P. Berrange wrote: > > On Tue, Apr 18, 2017 at 08:02:35AM -0400, Charles Bancroft wrote: > > > 2017-04-18 11:46:23.879+: 1: debug : virNetClientMarkClose:776 : > > > client=00faa9a0, reason=1 > > > > We're marking the conneciton as closed, due to EOF. > > > > This is the first sign of something wrong. > > > > I'm not 100% sure if this is due to the server closing the > > connection, or a client side bug in the poll loop. > > > > I'd be interested to see the libvirtd server side logs - if you configure > > libvirtd.conf with > > > > > > log_outputs="1:file:/var/log/libvirt/libvirtd.log" > > log_filters="1:remote 1:rpc 1:event 1:daemon" > > Actually don't bother - I just managed to reproduce the same problem myself > in a Windows machine. So this looks like a bug in libvirt on Win32. FYI, I've tracked this down to a likely bug in gnulib. All versions of libvirt from 1.2.14 onwards are affected & so will fail to connect. If you don't need the newer APIs, you could try using libvirt 1.2.13 for your Windows builds - I've tested that at least successfully connects. Meanwhile we'll aim to get this fixed in the next release (v3.3.0) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] hyperv: recognize array property as distinct type.
When hyperv code generator for WMI classes identifies common
properties, it needs to take into account array type as a distinct
type, i.e string != string[]. This is the case where v1 of the
Msvm_VirtualSystemSettingData has Notes property as string whereas v2
uses Notes[], therefore they have to be treated as different fields and
cannot be placed in the "common" struct.
---
changes in v3:
* add virBufferCheckError before calling virBufferContentAndReset.
src/hyperv/hyperv_driver.c | 28 ++--
src/hyperv/hyperv_wmi_generator.py | 2 +-
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 090ea24..8e5eeda 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -894,8 +894,32 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int
flags)
if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0)
goto cleanup;
-if (VIR_STRDUP(def->description,
virtualSystemSettingData->data.common->Notes) < 0)
-goto cleanup;
+if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
+if (VIR_STRDUP(def->description,
+ virtualSystemSettingData->data.v1->Notes) < 0)
+goto cleanup;
+} else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 &&
+ virtualSystemSettingData->data.v2->Notes.data != NULL) {
+char **notes = (char **) virtualSystemSettingData->data.v2->Notes.data;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+size_t i = 0;
+
+/* in practice Notes has 1 element */
+for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++) {
+/* but if there's more than 1, separate by double new line */
+if (virBufferUse(&buf) > 0)
+virBufferAddLit(&buf, "\n\n");
+
+virBufferAdd(&buf, *notes, -1);
+notes++;
+}
+
+if (virBufferCheckError(&buf))
+cleanup;
+
+def->description = virBufferContentAndReset(&buf);
+}
+
virDomainDefSetMemoryTotal(def, memorySettingData->data.common->Limit *
1024); /* megabyte to kilobyte */
def->mem.cur_balloon = memorySettingData->data.common->VirtualQuantity *
1024; /* megabyte to kilobyte */
diff --git a/src/hyperv/hyperv_wmi_generator.py
b/src/hyperv/hyperv_wmi_generator.py
index c15d97a..9aee0b9 100755
--- a/src/hyperv/hyperv_wmi_generator.py
+++ b/src/hyperv/hyperv_wmi_generator.py
@@ -251,7 +251,7 @@ class WmiClass:
for cls in self.versions:
for prop in cls.properties:
# consdered same if matches by name AND type
-key = "%s_%s" % (prop.name, prop.type)
+key = "%s_%s_%s" % (prop.name, prop.type, prop.is_array)
if key in property_info:
property_info[key][1] += 1
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] util: switch over to use keycodemapdb GIT submodule
On Wed, 2017-04-12 at 10:01 +0200, Andrea Bolognani wrote:
> > > I'm also wondering whether we can avoid having all developers
> > > run 'git submodule init && git submodule update' after these
> > > changes have been pushed...
> >
> > I assumed our blackmagic that deals with gnulib changing would
> > also pick up this addition and so make it 'just work', but
> > admittedly i've not tested that.
>
> It doesn't, but I'm adding some more black magic and it
> will once I'm done ;)
I just posted the autogen.sh changes[1], and I'm attaching a
tweaked version of your patch that builds on top of them to
make everything Just Work™.
[1] https://www.redhat.com/archives/libvir-list/2017-April/msg00816.html
--
Andrea Bolognani / Red Hat / VirtualizationFrom 8b2f6d86884fb2d5fdceaa754a8fe3301ca00ec6 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange"
Date: Fri, 3 Mar 2017 16:54:34 +
Subject: [PATCH] util: switch over to use keycodemapdb GIT submodule
A long time ago we imported the keymaps.csv file from GTK-VNC so we
can do conversions between keycode sets. Meanwhile lots of bug fixes
have gone into this CSV file and libvirt hasn't kept in sync. The
keymaps.csv file and associated generator script has been pulled out
of GTK-VNC into a dedicated GIT repo for use as a submodule. This
allows GTK-VNC, SPICE-GTK, QEMU and libvirt to share the same master
database and tools and pushing updates merely requires a submodule
commit update as with gnulib.
The test suite is updated to cover some extra boundary conditions.
Signed-off-by: Daniel P. Berrange
---
.gitignore | 3 +-
.gitmodules| 3 +
.submodules/keycodemapdb.functions | 47
src/Makefile.am| 45 +++-
src/keycodemapdb | 1 +
src/util/keymaps.csv | 464 -
src/util/virkeycode-mapgen.py | 97
src/util/virkeycode.c | 94
tests/virkeycodetest.c | 5 +
9 files changed, 136 insertions(+), 623 deletions(-)
create mode 100644 .submodules/keycodemapdb.functions
create mode 16 src/keycodemapdb
delete mode 100644 src/util/keymaps.csv
delete mode 100755 src/util/virkeycode-mapgen.py
diff --git a/.gitignore b/.gitignore
index f1ca602..bb9cd86 100644
--- a/.gitignore
+++ b/.gitignore
@@ -159,7 +159,8 @@
/src/test_libvirt*.aug
/src/test_virtlockd.aug
/src/test_virtlogd.aug
-/src/util/virkeymaps.h
+/src/util/virkeycodetable*.h
+/src/util/virkeynametable*.h
/src/virt-aa-helper
/src/virtlockd
/src/virtlogd
diff --git a/.gitmodules b/.gitmodules
index 7acb1ea..cfee24d 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +1,6 @@
[submodule "gnulib"]
path = .gnulib
url = git://git.sv.gnu.org/gnulib.git
+[submodule "keycodemapdb"]
+ path = src/keycodemapdb
+ url = https://gitlab.com/keycodemap/keycodemapdb.git
diff --git a/.submodules/keycodemapdb.functions b/.submodules/keycodemapdb.functions
new file mode 100644
index 000..615f8ff
--- /dev/null
+++ b/.submodules/keycodemapdb.functions
@@ -0,0 +1,47 @@
+keycodemapdb_hash()
+{
+local no_git=$1
+
+if test "$no_git"; then
+echo "no-git"
+return
+fi
+
+git submodule status src/keycodemapdb | awk '{ print $1 }'
+}
+
+keycodemapdb_update_required()
+{
+local expected=$1
+local actual=$2
+local no_git=$3
+
+local ret=0
+
+if test "$actual" = "$expected"; then
+ret=1
+fi
+
+return "$ret"
+}
+
+keycodemapdb_update()
+{
+local expected=$1
+local actual=$2
+local no_git=$3
+
+local ret=0
+
+if keycodemapdb_update_required "$expected" "$actual" "$no_git"; then
+echo "Updating keycodemapdb submodule..."
+if ! git submodule init src/keycodemapdb || \
+ ! git submodule update src/keycodemapdb; then
+ret=1
+fi
+fi
+
+return "$ret"
+}
+
+# vim: set syntax=sh:
diff --git a/src/Makefile.am b/src/Makefile.am
index e2dc77e..9282019 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -52,7 +52,7 @@ AM_LDFLAGS = $(DRIVER_MODULES_LDFLAGS) \
$(MINGW_EXTRA_LDFLAGS)\
$(NULL)
-EXTRA_DIST = $(conf_DATA) util/keymaps.csv
+EXTRA_DIST = $(conf_DATA)
BUILT_SOURCES =
CLEANFILES =
@@ -135,7 +135,6 @@ UTIL_SOURCES = \
util/virjson.c util/virjson.h \
util/virkeycode.c util/virkeycode.h \
util/virkeyfile.c util/virkeyfile.h \
- util/virkeymaps.h\
util/virlease.c util/virlease.h \
util/virlockspace.c util/virlockspace.h \
util/virlog.c util/virlog.h \
@@ -195,16 +194,38 @@ UTIL_SOURCES = \
util/virmdev.c util/virmdev.h \
$(NULL)
-EXTRA_DIST += $(srcdir)/util/keymaps.csv $(srcdir)/util/virkeycode-mapgen.py
-
-BUILT_SOURCES += util/virkeymaps.h
-MAINTAINERCLEANFILES += util/virkeymaps.h
-
-util/virkeymaps.h: $(srcdir)/util/keymaps.csv \
- $(srcdir)/util/virkeycode-mapgen.py
- $(MKDIR_P) util/
- $(AM_V_GEN)$(PYTHON) $(srcdir)/util/virke
[libvirt] [PATCH 0/2] autogen.sh: Rewrite and modularize
This work was motivated by Dan's intention to introduce the keycodemapdb module into libvirt[1]. As it is, we have a few useful submodule-related features in our build system, but they are limited in that they assume there will be a single submodule, and it will be gnulib. This series removes such assumptions, making it possible to introduce more submodules without having to sacrifice any convenience. [1] https://www.redhat.com/archives/libvir-list/2017-March/msg00136.html Andrea Bolognani (2): autogen.sh: Rewrite autogen.sh: Modularize .gitignore | 2 +- .submodules/gnulib.functions | 65 ++ autogen.sh | 286 +-- cfg.mk | 34 ++--- 4 files changed, 265 insertions(+), 122 deletions(-) create mode 100644 .submodules/gnulib.functions -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] Add support for QEMU's l3-cache CPU property
Hi, Apparently, reporting a level 3 cache on a virtual CPU can dramatically increase performance in some use cases [1]. The interesting part is that l3-cache=on does not provide the real CPU cache data, it's just making it up. Anyway, we should be able to enable this via libvirt. And since there is another property which enables real CPU cache data to be passed to a guest, I suggest the following /domain/cpu/cache element equivalent to l3-cache=on: If we need to add support for passing the real CPU cache data, we can do that with or we can even make the level attribute optional and support Missing cache element means default behaviour of the hypervisor and we can eventually add to turn off passing any CPU cache info to the guest. But I think we should now focus only on and leaving the rest for the future when we actually need it. This is how a more complete example would look like: Broadwell And libvirt would translate it into -cpu Broadwell,l3-cache=on. Do you have any thoughts about the XML schema or naming? Thanks, Jirka [1] https://patchwork.kernel.org/patch/9308401/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] autogen.sh: Modularize
This sets up a generic framework for dealing with git
submodules, removing the assumption that gnulib is going to
be the only git submodule libvirt uses and making it
relatively easy for more to be added without sacrificing
the convenience we've gotten used to, eg. automatic updates.
---
.gitignore | 2 +-
.submodules/gnulib.functions | 65 ++
autogen.sh | 155 ++-
3 files changed, 145 insertions(+), 77 deletions(-)
create mode 100644 .submodules/gnulib.functions
diff --git a/.gitignore b/.gitignore
index d7927e1..f1ca602 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,11 +30,11 @@
.dirstamp
.gdb_history
.git
-.git-module-status
.libs
.lvimrc
.memdump
.sc-start-sc_*
+.submodules/*.status
/ABOUT-NLS
/AUTHORS
/ChangeLog
diff --git a/.submodules/gnulib.functions b/.submodules/gnulib.functions
new file mode 100644
index 000..ec89b37
--- /dev/null
+++ b/.submodules/gnulib.functions
@@ -0,0 +1,65 @@
+gnulib_hash()
+{
+local no_git=$1
+
+if test "$no_git"; then
+echo "no-git"
+return
+fi
+
+# Compute the hash we'll use to determine whether rerunning bootstrap
+# is required. The first is just the SHA1 that selects a gnulib snapshot.
+# The second ensures that whenever we change the set of gnulib modules used
+# by this package, we rerun bootstrap to pull in the matching set of files.
+# The third ensures that whenever we change the set of local gnulib diffs,
+# we rerun bootstrap to pull in those diffs.
+git submodule status .gnulib | awk '{ print $1 }'
+git hash-object bootstrap.conf
+git ls-tree -d HEAD gnulib/local | awk '{ print $3 }'
+}
+
+gnulib_update_required()
+{
+local expected=$1
+local actual=$2
+local no_git=$3
+
+local ret=0
+
+# Whenever the gnulib submodule or any of the related bits has been
+# changed in some way (see gnulib_hash) we need to update the submodule,
+# eg. run bootstrap again; updating is also needed if any of the files
+# that can only be generated through bootstrap has gone missing
+if test "$actual" = "$expected" && \
+ test -f po/Makevars && test -f AUTHORS; then
+ret=1
+fi
+
+return "$ret"
+}
+
+gnulib_update()
+{
+local expected=$1
+local actual=$2
+local no_git=$3
+
+local ret=0
+
+# Depending on whether or not an update is required, we might be able to
+# get away with simply running autoreconf, or we might have to go through
+# the bootstrap process
+if gnulib_update_required "$expected" "$actual" "$no_git"; then
+echo "Running bootstrap..."
+./bootstrap$no_git --bootstrap-sync
+ret=$?
+else
+echo "Running autoreconf..."
+autoreconf -if
+ret=$?
+fi
+
+return "$ret"
+}
+
+# vim: set syntax=sh:
diff --git a/autogen.sh b/autogen.sh
index 98baab7..38b3476 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -7,6 +7,19 @@ die()
exit 1
}
+git_submodules()
+{
+# Print the list of known submodules. Parse the .gitmodules file because
+# we want the submodule's alias rather than its path (as we're going to
+# use it for building eg. function names) and the 'git submodule' command
+# doesn't provide us with that information
+grep -E '^\[submodule ' .gitmodules | while read _ name; do
+name=${name#\"}
+name=${name%%\"*}
+echo "$name"
+done
+}
+
starting_point=$(pwd)
srcdir=$(dirname "$0")
@@ -68,70 +81,6 @@ while test "$#" -gt 0; do
done
no_git="$no_git$gnulib_srcdir"
-gnulib_hash()
-{
-local no_git=$1
-
-if test "$no_git"; then
-echo "no-git"
-return
-fi
-
-# Compute the hash we'll use to determine whether rerunning bootstrap
-# is required. The first is just the SHA1 that selects a gnulib snapshot.
-# The second ensures that whenever we change the set of gnulib modules used
-# by this package, we rerun bootstrap to pull in the matching set of files.
-# The third ensures that whenever we change the set of local gnulib diffs,
-# we rerun bootstrap to pull in those diffs.
-git submodule status .gnulib | awk '{ print $1 }'
-git hash-object bootstrap.conf
-git ls-tree -d HEAD gnulib/local | awk '{ print $3 }'
-}
-
-gnulib_update_required()
-{
-local expected=$1
-local actual=$2
-local no_git=$3
-
-local ret=0
-
-# Whenever the gnulib submodule or any of the related bits has been
-# changed in some way (see gnulib_hash) we need to update the submodule,
-# eg. run bootstrap again; updating is also needed if any of the files
-# that can only be generated through bootstrap has gone missing
-if test "$actual" = "$expected" && \
- test -f po/Makevars && test -f AUTHORS; then
-ret=1
-fi
-
-return "$ret"
-}
-
-gnulib_update()
-{
-local expected=$1
-local actual=$2
-local no_git=$3
-
-lo
[libvirt] [PATCH 1/2] autogen.sh: Rewrite
The goal is twofold: firstly, we want to be able to make parts
of it into reusable modules, and secondly we'd like to reduce
the amount of duplicated code. Moreover, since we're making
heavy changes to the code anyway, we might as well make sure
it follows a somewhat consistent coding style too.
To reduce code duplication, we introduce a new --dry-run
option, which can be used by third parties to figure out
whether calling autogen.sh is necessary or not: this allows
us to get rid of the reimplementation of part of the logic
in cfg.mk and guarantee they'll never get out of sync.
In order to be able to move forward with modularization down
the line, we extract the gnulib-specific part of the submodule
logic into three functions which adhere to a fairly generic
API that's not tied to gnulib.
Other changes include: removing assumptions about gnulib being
the only submodule, both in actual logic and in messages to
the user; making dirty submodules checking and cleaning
entirely independent of whether or not updating submodules is
needed; removing the use of 'set -e' and handling errors
explicitly instead.
---
I originally intended to proceed with this rewrite through
a series of small, incremental changes; however, I soon
realized it would take a lot more time and energy, and the
result will be in some case quite frankly silly, so I've
decided to go with a single commit that includes an extensive
explanation of goals and changes. Basically: the diff is
useless, just look at the two scripts side by side ;)
The shell code is probably not 100% portable to all weird
platforms out there (the use of 'local' comes to mind), but
it should be portable enough to work on all those we target
with libvirt. I've tested it on Fedora 24, CentOS 6 and
FreeBSD 11 without running into any issue.
autogen.sh | 269 +
cfg.mk | 34 ++--
2 files changed, 189 insertions(+), 114 deletions(-)
diff --git a/autogen.sh b/autogen.sh
index d1c319d..98baab7 100755
--- a/autogen.sh
+++ b/autogen.sh
@@ -1,117 +1,210 @@
#!/bin/sh
# Run this to generate all the initial makefiles, etc.
-set -e
+die()
+{
+echo "error: $1" >&2
+exit 1
+}
-srcdir=`dirname "$0"`
-test -z "$srcdir" && srcdir=.
+starting_point=$(pwd)
-THEDIR=`pwd`
-cd "$srcdir"
+srcdir=$(dirname "$0")
+test "$srcdir" || srcdir=.
-test -f src/libvirt.c || {
-echo "You must run this script in the top-level libvirt directory"
-exit 1
+cd "$srcdir" || {
+die "failed to cd into $srcdir"
}
+test -f src/libvirt.c || {
+die "$0 must live in the top-level libvirt directory"
+}
-EXTRA_ARGS=
+dry_run=
no_git=
-if test "x$1" = "x--no-git"; then
- no_git=" $1"
- shift
- case "$1 $2" in
---gnulib-srcdir=*) no_git="$no_git $1"; shift ;;
---gnulib-srcdir\ *) no_git="$no_git $1=$2"; shift; shift;;
- esac
-fi
-if test -z "$NOCONFIGURE" ; then
- if test "x$1" = "x--system"; then
-shift
-prefix=/usr
-libdir=$prefix/lib
-sysconfdir=/etc
-localstatedir=/var
-if [ -d /usr/lib64 ]; then
- libdir=$prefix/lib64
-fi
-EXTRA_ARGS="--prefix=$prefix --sysconfdir=$sysconfdir
--localstatedir=$localstatedir --libdir=$libdir"
-echo "Running ./configure with $EXTRA_ARGS $@"
- else
-if test -z "$*" && test ! -f "$THEDIR/config.status"; then
-echo "I am going to run ./configure with no arguments - if you wish"
-echo "to pass any to it, please specify them on the $0 command line."
-fi
- fi
-fi
+gnulib_srcdir=
+extra_args=
+while test "$#" -gt 0; do
+case "$1" in
+--dry-run)
+# This variable will serve both as an indicator of the fact that a
+# dry run has been requested, and to store the result of the dry run.
+# It will be ultimately used as return code for the script, so a
+# value of 1 means "running autogen.sh is not needed at this time"
+dry_run=1
+shift
+;;
+--no-git)
+no_git=" $1"
+shift
+;;
+--gnulib-srcdir=*)
+gnulib_srcdir=" $1"
+shift
+;;
+--gnulib-srcdir)
+gnulib_srcdir=" $1=$2"
+shift
+shift
+;;
+--system)
+prefix=/usr
+sysconfdir=/etc
+localstatedir=/var
+if test -d $prefix/lib64; then
+libdir=$prefix/lib64
+else
+libdir=$prefix/lib
+fi
+extra_args="--prefix=$prefix --localstatedir=$localstatedir"
+extra_args="$extra_args --sysconfdir=$sysconfdir --libdir=$libdir"
+shift
+;;
+*)
+# All remaining arguments will be passed to configure verbatim
+break
+;;
+esac
+done
+no_git="$no_git$gnulib_srcdir"
-# Compute the hash we'll use to determine whether rerunning bootstrap
-# is required. The first is just the SHA1 that selects a gnulib snapshot.
-# The second ensures that whenever we change the set of gnulib modules used
-# by this
[libvirt] [PATCH] [PATCH] rpc: fix keep alive timer segfault
ka maybe have been freeed in virObjectUnref, application using
virKeepAliveTimer will segfault when unlock ka. We should keep
ka's refs positive before using it.
Signed-off-by: Yi Wang
---
src/rpc/virkeepalive.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c
index c9faf88..4f666fd 100644
--- a/src/rpc/virkeepalive.c
+++ b/src/rpc/virkeepalive.c
@@ -160,17 +160,17 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void
*opaque)
bool dead;
void *client;
+virObjectRef(ka);
virObjectLock(ka);
client = ka->client;
dead = virKeepAliveTimerInternal(ka, &msg);
+virObjectUnlock(ka);
+
if (!dead && !msg)
goto cleanup;
-virObjectRef(ka);
-virObjectUnlock(ka);
-
if (dead) {
ka->deadCB(client);
} else if (ka->sendCB(client, msg) < 0) {
@@ -178,11 +178,8 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void *opaque)
virNetMessageFree(msg);
}
-virObjectLock(ka);
-virObjectUnref(ka);
-
cleanup:
-virObjectUnlock(ka);
+virObjectUnref(ka);
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [REPOST PATCH 0/5] Split out network object into its own module
On 04/13/2017 02:04 AM, John Ferlan wrote: Reposting with update to top of git tree commit id '0563d3f06' from: https://www.redhat.com/archives/libvir-list/2017-March/msg00892.html Previous cover letter - cut-n-pasted: Reached the last of the code from my RFC for making a common pool object - networkobj... For reference see patch 3 of: http://www.redhat.com/archives/libvir-list/2017-February/msg00519.html This series works through the network conf adjustments. This pile is fairly straightforward, like other series we split out anything referencing virNetworkPoolObj and go from there. There was one small detour to avoid a potentially conflict in names. John Ferlan (5): conf: Introduce virnetworkobj conf: Adjust coding style for network conf sources conf: Alter coding style of network conf function prototypes conf: Rename virNetworkObjAssignDef to virNetworkObjUpdateAssignDef conf: Use consistent function name prefixes for virnetworkobj po/POTFILES.in |1 + src/Makefile.am |3 +- src/conf/network_conf.c | 1388 ++ src/conf/network_conf.h | 204 ++--- src/conf/virnetworkobj.c | 1372 + src/conf/virnetworkobj.h | 185 + src/libvirt_private.syms | 57 +- src/network/bridge_driver.c | 62 +- src/network/bridge_driver.h |2 +- src/network/bridge_driver_platform.h |2 +- src/test/test_driver.c | 16 +- 11 files changed, 1763 insertions(+), 1529 deletions(-) create mode 100644 src/conf/virnetworkobj.c create mode 100644 src/conf/virnetworkobj.h ACK series. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] server side copy from one volume to another
On Tuesday, 18 April 2017 13:03:11 CEST Vasiliy Tolstov wrote: > 2017-04-18 8:39 GMT+03:00 Michal Privoznik : > > s that right? Here I'm successfully cloning a volume across two storage > > pools: > > > Yes, it works but > Source image: > image: /var/lib/libvirt/images/sda/143177 > file format: qcow2 > virtual size: 5.0G (5368709120 bytes) > disk size: 902M > cluster_size: 65536 > Format specific information: > compat: 0.10 > refcount bits: 16 > > Dest image: > image: /var/lib/libvirt/images/sda/test > file format: qcow2 > virtual size: 5.0G (5368709120 bytes) > disk size: 902M > cluster_size: 65536 > Format specific information: > compat: 0.10 > refcount bits: 16 > > > But > -rw--- 1 root 908M Apr 18 13:59 143177 > -rw--- 1 root 902M Apr 18 13:59 test > > Why size is changed on destination image? In case of cp > --spwarse=always size does not changed. As Michal and Daniel said, this should not be a problem. Also, you can check the content is the same using virt-diff (part of libguestfs): # virt-diff \ -a /var/lib/libvirt/images/sda/143177 \ -A /var/lib/libvirt/images/sda/test > log Most probably log should be empty, since the actual content of the image did not change. -- Pino Toscano signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] error in docs https://libvirt.org/html/libvirt-libvirt-domain.html
virDomainMigrateFlags have errors VIR_MIGRATE_LIVE have invalid comment and as i see all comments are shifted down by one point. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Trouble with virsh on Windows
On Tue, Apr 18, 2017 at 01:41:27PM +0100, Daniel P. Berrange wrote: > On Tue, Apr 18, 2017 at 08:02:35AM -0400, Charles Bancroft wrote: > > 2017-04-18 11:46:23.879+: 1: debug : virNetClientMarkClose:776 : > > client=00faa9a0, reason=1 > > We're marking the conneciton as closed, due to EOF. > > This is the first sign of something wrong. > > I'm not 100% sure if this is due to the server closing the > connection, or a client side bug in the poll loop. > > I'd be interested to see the libvirtd server side logs - if you configure > libvirtd.conf with > > > log_outputs="1:file:/var/log/libvirt/libvirtd.log" > log_filters="1:remote 1:rpc 1:event 1:daemon" Actually don't bother - I just managed to reproduce the same problem myself in a Windows machine. So this looks like a bug in libvirt on Win32. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Trouble with virsh on Windows
On Tue, Apr 18, 2017 at 08:02:35AM -0400, Charles Bancroft wrote: > > > On 4/18/2017 4:48 AM, Daniel P. Berrange wrote: > > On Mon, Apr 17, 2017 at 09:49:01AM -0400, Charles Bancroft wrote: > >> Hi List, > >> > >> I have a question about some troubles I am having with virsh on Windows > >> x64. I am currently running a KVM server on a linux box and need to > >> allow Windows clients to access it. I have set up the server for access > >> with: > >> > >> ``` > >> listen_tls = 0 > >> listen_tcp = 1 > >> auth_tcp = "none" > Here is the log of my execution run. Looks like there was an error on > the socket, but nothing useful came back. I have also attached a > tcpdump log of the network traffic > to show it is the client end(192.168.1.10) issuing the reset after > receiving some data from the server (192.168.1.113) > ``` Execution log > $ LIBVIRT_DEBUG=1 ./virsh -c qemu+tcp://192.168.1.113/system -d0 > 2017-04-18 11:46:23.778+: 1: info : libvirt version: 3.1.0 > 2017-04-18 11:46:23.778+: 1: info : hostname: Is '' /really/ your local hostname, or did you just edit the logs to hide your hostname ? > 2017-04-18 11:46:23.790+: 1: debug : doRemoteOpen:907 : proceeding > with name = qemu:///system > 2017-04-18 11:46:23.790+: 1: debug : doRemoteOpen:916 : Connecting > with transport 5 Ok, we're trying to connect to the host over TCP which is good > 2017-04-18 11:46:23.875+: 1: debug : virNetSocketNew:235 : > localAddr=0069f540 remoteAddr=0069f5d0 fd=5 errfd=-1 pid=0 > 2017-04-18 11:46:23.875+: 1: info : virObjectNew:202 : OBJECT_NEW: > obj=00faebe0 classname=virNetSocket > 2017-04-18 11:46:23.875+: 1: info : virNetSocketNew:291 : > RPC_SOCKET_NEW: sock=00faebe0 fd=5 errfd=-1 pid=0 > localAddr=192.168.1.10;61441, remoteAddr=192.168.1.113;16509 Ok, the TCP connection is successfull, which is also good. > 2017-04-18 11:46:23.875+: 1: debug : doRemoteOpen:1170 : Trying > authentication We're now trying to authenticate with the server > 2017-04-18 11:46:23.875+: 1: debug : virNetMessageNew:46 : > msg=00fba700 tracked=0 > 2017-04-18 11:46:23.875+: 1: debug : virNetMessageEncodePayload:386 > : Encode length as 28 > 2017-04-18 11:46:23.875+: 1: info : virNetClientSendInternal:2104 : > RPC_CLIENT_MSG_TX_QUEUE: client=00faa9a0 len=28 prog=536903814 > vers=1 proc=66 type=0 status=0 serial=0 > 2017-04-18 11:46:23.875+: 1: debug : virNetClientCallNew:2057 : New > call 00fd7ad0: msg=00fba700, expectReply=1, nonBlock=0 > 2017-04-18 11:46:23.875+: 1: debug : virNetClientIO:1866 : Outgoing > message prog=536903814 version=1 serial=0 proc=66 type=0 length=28 > dispatch= We've put the authentication message on the queue to send. > 2017-04-18 11:46:23.879+: 1: debug : virNetClientMarkClose:776 : > client=00faa9a0, reason=1 We're marking the conneciton as closed, due to EOF. This is the first sign of something wrong. I'm not 100% sure if this is due to the server closing the connection, or a client side bug in the poll loop. I'd be interested to see the libvirtd server side logs - if you configure libvirtd.conf with log_outputs="1:file:/var/log/libvirt/libvirtd.log" log_filters="1:remote 1:rpc 1:event 1:daemon" > error: failed to connect to the hypervisor > error: An error occurred, but the cause is unknown This however is a clear bug - we should never get this particular error message. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] server side copy from one volume to another
2017-04-18 15:00 GMT+03:00 Daniel P. Berrange : > XFS will also do speculative allocation of filesystem blocks which have > not yet had data written to them, on the basis that a future write might > need them soon. So you can't really compare allocation disk size at all. Ok thanks for info and for saving my time =)! -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/38] virfdstream: Use messages instead of pipe
On 04/18/2017 02:03 PM, Daniel P. Berrange wrote:
On Tue, Apr 18, 2017 at 02:00:09PM +0200, Michal Privoznik wrote:
On 04/13/2017 07:13 PM, Daniel P. Berrange wrote:
On Thu, Apr 13, 2017 at 06:52:31PM +0200, Michal Privoznik wrote:
On 04/13/2017 03:55 PM, Daniel P. Berrange wrote:
On Thu, Apr 13, 2017 at 03:31:14PM +0200, Michal Privoznik wrote:
One big downside of using the pipe to transfer the data is that
we can really transfer just bare data. No metadata can be carried
through unless some formatted messages are introduced. That would
be quite painful to achieve so let's use a message queue. It's
fairly easy to exchange info between threads now that iohelper is
no longer used.
I'm not seeing how this works correctly with the event loop.
@@ -752,8 +1014,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
if ((st->flags & VIR_STREAM_NONBLOCK) &&
((!S_ISCHR(sb.st_mode) &&
!S_ISFIFO(sb.st_mode)) || forceIOHelper)) {
-int fds[2] = { -1, -1 };
-
if ((oflags & O_ACCMODE) == O_RDWR) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: Cannot request read and write flags
together"),
@@ -761,12 +1021,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
goto error;
}
-if (pipe(fds) < 0) {
-virReportSystemError(errno, "%s",
- _("Unable to create pipe"));
-goto error;
-}
Here we previously created the pipe
@@ -775,18 +1029,14 @@ virFDStreamOpenFileInternal(virStreamPtr st,
if ((oflags & O_ACCMODE) == O_RDONLY) {
threadData->fdin = fd;
-threadData->fdout = fds[1];
-if (VIR_STRDUP(threadData->fdinname, path) < 0 ||
-VIR_STRDUP(threadData->fdoutname, "pipe") < 0)
+threadData->fdout = -1;
+if (VIR_STRDUP(threadData->fdinname, path) < 0)
goto error;
-fd = fds[0];
And here we set 'fd' to be the pipe
} else {
-threadData->fdin = fds[0];
+threadData->fdin = -1;
threadData->fdout = fd;
-if (VIR_STRDUP(threadData->fdinname, "pipe") < 0 ||
-VIR_STRDUP(threadData->fdoutname, path) < 0)
+if (VIR_STRDUP(threadData->fdoutname, path) < 0)
goto error;
-fd = fds[1];
Likewise here
}
}
...now here 'fd' is passed to virFDStreamOpenInternal() and is the thing
that the event loop watches are registered against by virFDStreamAddCallback
With this change 'fd' is the actual plain file the thread is reading to/from,
so the callbacks are being registered against the plain file, not the pipe.
poll/select on POSIX always reports plain files as readable/writable even
when they would block. So with this change we're just going to busy loop
in the main event thread even when we'll block, which defeats the whole
purpose of having a iohelper and/or thread.
Oh, I've misunderstood what we've discussed on IRC then. The way I've
understood it was that if an FD is set to nonblock mode and poll()
claims there are some data available, subsequent read() might block. If
that was the case we would be safe with this code. However, I didn't
expect poll() to lie.
This code wouldn't be safe - anytime poll claims data available, we *must*
be able to read without blocking.
Any link for further reading on this? I guess it's not only us who has
to deal with this problem. Basically any application with poll() and
disk read()/write() has to suffer from this.
Yes, that's correct - QEMU has the same issue for example - it is why there
is no 'file:' protocol for migration for example - it would block the QEMU
main loop.
So what are our options here? Because I don't see any right now.
IIUC, you didn't want to use a pipe because you want to send structured
messages, not just plain data. If we just have a linked list of messages
there's nothing we can poll on, so we need to keep the pipe in use, but
find a way to get the special messages in the flow.
I think we could do a trick where we have two pipes in use, one for
monitoring the readability, and one for monitoring writability.
When the I/O thread has data on the queue ready for read by the main
thread, it can write a single byte to the read-monitor pipe.
When the I/O thread is ready to accept more data to write from the
main thread, it can write a single byte to the write-monitor pipe.
The main thread would monitor for POLLIN condition on both the
read-monitor pipe and write-monitor pipe.
Ah, indeed. This could work. But I also thought over different approach.
What I need really is transfer "you're in a data/hole X bytes long" besides
actual data. So I can use pipe for transferring the data as is currently,
and store the metadata into a structured message that would the thread
write/read and event loop read/write.
Sure, that works too. Just depends how much you ca
Re: [libvirt] [PATCH 06/38] virfdstream: Use messages instead of pipe
On Tue, Apr 18, 2017 at 02:00:09PM +0200, Michal Privoznik wrote:
> On 04/13/2017 07:13 PM, Daniel P. Berrange wrote:
> > On Thu, Apr 13, 2017 at 06:52:31PM +0200, Michal Privoznik wrote:
> > > On 04/13/2017 03:55 PM, Daniel P. Berrange wrote:
> > > > On Thu, Apr 13, 2017 at 03:31:14PM +0200, Michal Privoznik wrote:
> > > > > One big downside of using the pipe to transfer the data is that
> > > > > we can really transfer just bare data. No metadata can be carried
> > > > > through unless some formatted messages are introduced. That would
> > > > > be quite painful to achieve so let's use a message queue. It's
> > > > > fairly easy to exchange info between threads now that iohelper is
> > > > > no longer used.
> > > >
> > > > I'm not seeing how this works correctly with the event loop.
> > > >
> > > > > @@ -752,8 +1014,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
> > > > > if ((st->flags & VIR_STREAM_NONBLOCK) &&
> > > > > ((!S_ISCHR(sb.st_mode) &&
> > > > >!S_ISFIFO(sb.st_mode)) || forceIOHelper)) {
> > > > > -int fds[2] = { -1, -1 };
> > > > > -
> > > > > if ((oflags & O_ACCMODE) == O_RDWR) {
> > > > > virReportError(VIR_ERR_INTERNAL_ERROR,
> > > > > _("%s: Cannot request read and write
> > > > > flags together"),
> > > > > @@ -761,12 +1021,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
> > > > > goto error;
> > > > > }
> > > > >
> > > > > -if (pipe(fds) < 0) {
> > > > > -virReportSystemError(errno, "%s",
> > > > > - _("Unable to create pipe"));
> > > > > -goto error;
> > > > > -}
> > > >
> > > > Here we previously created the pipe
> > > >
> > > > > @@ -775,18 +1029,14 @@ virFDStreamOpenFileInternal(virStreamPtr st,
> > > > >
> > > > > if ((oflags & O_ACCMODE) == O_RDONLY) {
> > > > > threadData->fdin = fd;
> > > > > -threadData->fdout = fds[1];
> > > > > -if (VIR_STRDUP(threadData->fdinname, path) < 0 ||
> > > > > -VIR_STRDUP(threadData->fdoutname, "pipe") < 0)
> > > > > +threadData->fdout = -1;
> > > > > +if (VIR_STRDUP(threadData->fdinname, path) < 0)
> > > > > goto error;
> > > > > -fd = fds[0];
> > > >
> > > > And here we set 'fd' to be the pipe
> > > >
> > > > > } else {
> > > > > -threadData->fdin = fds[0];
> > > > > +threadData->fdin = -1;
> > > > > threadData->fdout = fd;
> > > > > -if (VIR_STRDUP(threadData->fdinname, "pipe") < 0 ||
> > > > > -VIR_STRDUP(threadData->fdoutname, path) < 0)
> > > > > +if (VIR_STRDUP(threadData->fdoutname, path) < 0)
> > > > > goto error;
> > > > > -fd = fds[1];
> > > >
> > > > Likewise here
> > > >
> > > > > }
> > > > > }
> > > >
> > > > ...now here 'fd' is passed to virFDStreamOpenInternal() and is the thing
> > > > that the event loop watches are registered against by
> > > > virFDStreamAddCallback
> > > >
> > > >
> > > > With this change 'fd' is the actual plain file the thread is reading
> > > > to/from,
> > > > so the callbacks are being registered against the plain file, not the
> > > > pipe.
> > > >
> > > > poll/select on POSIX always reports plain files as readable/writable
> > > > even
> > > > when they would block. So with this change we're just going to busy
> > > > loop
> > > > in the main event thread even when we'll block, which defeats the whole
> > > > purpose of having a iohelper and/or thread.
> > >
> > > Oh, I've misunderstood what we've discussed on IRC then. The way I've
> > > understood it was that if an FD is set to nonblock mode and poll()
> > > claims there are some data available, subsequent read() might block. If
> > > that was the case we would be safe with this code. However, I didn't
> > > expect poll() to lie.
> >
> > This code wouldn't be safe - anytime poll claims data available, we *must*
> > be able to read without blocking.
> >
> > > Any link for further reading on this? I guess it's not only us who has
> > > to deal with this problem. Basically any application with poll() and
> > > disk read()/write() has to suffer from this.
> >
> > Yes, that's correct - QEMU has the same issue for example - it is why there
> > is no 'file:' protocol for migration for example - it would block the QEMU
> > main loop.
> >
> > > So what are our options here? Because I don't see any right now.
> >
> > IIUC, you didn't want to use a pipe because you want to send structured
> > messages, not just plain data. If we just have a linked list of messages
> > there's nothing we can poll on, so we need to keep the pipe in use, but
> > find a way to get the special messages in the flow.
> >
> > I think we could do a trick where we have two pipes in use, one for
> > monitoring the readability, and one for
Re: [libvirt] Trouble with virsh on Windows
On 4/18/2017 4:48 AM, Daniel P. Berrange wrote: > On Mon, Apr 17, 2017 at 09:49:01AM -0400, Charles Bancroft wrote: >> Hi List, >> >> I have a question about some troubles I am having with virsh on Windows >> x64. I am currently running a KVM server on a linux box and need to >> allow Windows clients to access it. I have set up the server for access >> with: >> >> ``` >> listen_tls = 0 >> listen_tcp = 1 >> auth_tcp = "none" >> ``` > I assume you're merely doing this for sake of testing / debugging. > > This opens libvirtd connections with no access control, which is > equivalent to running an SSH server allowing root logins with no > password. ie you never want to do this in real deployments. Yes, the final version will most certainly be using TLS or ssh. This was just a proof of concept to validate our code would work on windows. >> to allow for remote access while testing things. I have verified that I >> can connect remotely from other linux machines, but my windows machine >> always reports: >> >> ``` >> error: failed to connect to the hypervisor >> error: An error occurred, but the cause is unknown >> ``` >> >> On the server side I see: >> ``` >> 10435: error : virNetSocketReadWire:1793 : Cannot recv data: Connection >> reset by peer >> ``` >> >> This happens no matter what build of virsh for windows I am using. I >> have tried 3 different mingw-64 builds and versions and am about to just >> build my own from scratch. I have verified that it is not the firewall >> or the version of Windows I am using. I am also able to use virsh >> within a docker or from the Windows Subsystem for Linux bash prompt. >> >> Any ideas as to what could be causing this connection reset? It happens >> as soon as a connection attempt occurs and I am running out of idea. > When running virsh, set 'LIBVIRT_DEBUG=1' environment variable, so we > can see some debug logs, which might give a hint as to what is wrong. Here is the log of my execution run. Looks like there was an error on the socket, but nothing useful came back. I have also attached a tcpdump log of the network traffic to show it is the client end(192.168.1.10) issuing the reset after receiving some data from the server (192.168.1.113) ``` Execution log $ LIBVIRT_DEBUG=1 ./virsh -c qemu+tcp://192.168.1.113/system -d0 2017-04-18 11:46:23.778+: 1: info : libvirt version: 3.1.0 2017-04-18 11:46:23.778+: 1: info : hostname: 2017-04-18 11:46:23.778+: 1: debug : virGlobalInit:386 : register drivers 2017-04-18 11:46:23.782+: 1: debug : virRegisterConnectDriver:684 : driver=6cdc19e0 name=Test 2017-04-18 11:46:23.782+: 1: debug : virRegisterConnectDriver:695 : registering Test as driver 0 2017-04-18 11:46:23.782+: 1: debug : virRegisterConnectDriver:684 : driver=6cdc24e0 name=remote 2017-04-18 11:46:23.782+: 1: debug : virRegisterConnectDriver:695 : registering remote as driver 1 2017-04-18 11:46:23.790+: 1: debug : virEventRegisterDefaultImpl:267 : registering default event implementation 2017-04-18 11:46:23.790+: 1: debug : virEventPollAddHandle:115 : Used 0 handle slots, adding at least 10 more 2017-04-18 11:46:23.790+: 1: debug : virEventPollInterruptLocked:722 : Skip interrupt, 0 0 2017-04-18 11:46:23.790+: 1: info : virEventPollAddHandle:140 : EVENT_POLL_ADD_HANDLE: watch=1 fd=3 events=1 cb=6cc17d30 opaque= ff= 2017-04-18 11:46:23.790+: 1: debug : virEventRegisterImpl:234 : addHandle=6cc17d70 updateHandle=6cc17f80 removeHandle=6cc18100 addTimeout=6cc18280 updateTimeout=6cc18470 removeTimeout=6cc18640 2017-04-18 11:46:23.790+: 1: debug : virEventPollAddTimeout:230 : Used 0 timeout slots, adding at least 10 more 2017-04-18 11:46:23.790+: 1: debug : virEventPollInterruptLocked:722 : Skip interrupt, 0 0 2017-04-18 11:46:23.790+: 1: info : virEventPollAddTimeout:253 : EVENT_POLL_ADD_TIMEOUT: timer=1 frequency=-1 cb=00432da0 opaque=0069fbf0 ff= 2017-04-18 11:46:23.790+: 1: debug : virConnectOpenAuth:1245 : name=qemu+tcp://192.168.1.113/system, auth=6cdc1780, flags=0 2017-04-18 11:46:23.790+: 1: info : virObjectNew:202 : OBJECT_NEW: obj=00faeb10 classname=virConnect 2017-04-18 11:46:23.790+: 2: debug : virThreadJobSet:99 : Thread 2 is now running job vshEventLoop 2017-04-18 11:46:23.790+: 2: debug : virEventRunDefaultImpl:311 : running default event implementation 2017-04-18 11:46:23.790+: 2: debug : virEventPollCleanupTimeouts:525 : Cleanup 1 2017-04-18 11:46:23.790+: 1: debug : virConfGetValueStringList:981 : Get value string list 0 2017-04-18 11:46:23.790+: 2: debug : virEventPollCleanupHandles:574 : Cleanup 1 2017-04-18 11:46:23.790+: 2: debug : virEventPollMakePollFDs:401 : Prepare n=0 w=1, f=3 e=1 d=0 2017-04-18 11:46:23.790+: 1: debug : virConnectOpenInternal:1033 : Split "qemu+tcp://192.168.1.1
Re: [libvirt] server side copy from one volume to another
On Tue, Apr 18, 2017 at 01:31:11PM +0200, Michal Privoznik wrote: > On 04/18/2017 01:03 PM, Vasiliy Tolstov wrote: > > 2017-04-18 8:39 GMT+03:00 Michal Privoznik : > > > s that right? Here I'm successfully cloning a volume across two storage > > > pools: > > > > > > Yes, it works but > > Source image: > > image: /var/lib/libvirt/images/sda/143177 > > file format: qcow2 > > virtual size: 5.0G (5368709120 bytes) > > disk size: 902M > > cluster_size: 65536 > > Format specific information: > > compat: 0.10 > > refcount bits: 16 > > > > Dest image: > > image: /var/lib/libvirt/images/sda/test > > file format: qcow2 > > virtual size: 5.0G (5368709120 bytes) > > disk size: 902M > > cluster_size: 65536 > > Format specific information: > > compat: 0.10 > > refcount bits: 16 > > > > > > But > > -rw--- 1 root 908M Apr 18 13:59 143177 > > -rw--- 1 root 902M Apr 18 13:59 test > > > > Why size is changed on destination image? In case of cp > > --spwarse=always size does not changed. > > > > That's because libvirt does not use `cp` to copy the disk images. We use > qemu-img for copying. And we do not guarantee that the disk structure does > not change. We guarantee that the data stored on the disk doesn't change. > Otherwise we wouldn't be able to convert disk images. > > NB, with sparse files - it's not as straightforward either. Some filesystems > may decide that the hole is not big enough so they do allocate it actually. > XFS is a very good example of such behaviour. So the data/hole structure of > a file may not line up with the structure of a copy. Although, the data as > read from the files are the same. XFS will also do speculative allocation of filesystem blocks which have not yet had data written to them, on the basis that a future write might need them soon. So you can't really compare allocation disk size at all. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/38] virfdstream: Use messages instead of pipe
On 04/13/2017 07:13 PM, Daniel P. Berrange wrote:
On Thu, Apr 13, 2017 at 06:52:31PM +0200, Michal Privoznik wrote:
On 04/13/2017 03:55 PM, Daniel P. Berrange wrote:
On Thu, Apr 13, 2017 at 03:31:14PM +0200, Michal Privoznik wrote:
One big downside of using the pipe to transfer the data is that
we can really transfer just bare data. No metadata can be carried
through unless some formatted messages are introduced. That would
be quite painful to achieve so let's use a message queue. It's
fairly easy to exchange info between threads now that iohelper is
no longer used.
I'm not seeing how this works correctly with the event loop.
@@ -752,8 +1014,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
if ((st->flags & VIR_STREAM_NONBLOCK) &&
((!S_ISCHR(sb.st_mode) &&
!S_ISFIFO(sb.st_mode)) || forceIOHelper)) {
-int fds[2] = { -1, -1 };
-
if ((oflags & O_ACCMODE) == O_RDWR) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: Cannot request read and write flags
together"),
@@ -761,12 +1021,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
goto error;
}
-if (pipe(fds) < 0) {
-virReportSystemError(errno, "%s",
- _("Unable to create pipe"));
-goto error;
-}
Here we previously created the pipe
@@ -775,18 +1029,14 @@ virFDStreamOpenFileInternal(virStreamPtr st,
if ((oflags & O_ACCMODE) == O_RDONLY) {
threadData->fdin = fd;
-threadData->fdout = fds[1];
-if (VIR_STRDUP(threadData->fdinname, path) < 0 ||
-VIR_STRDUP(threadData->fdoutname, "pipe") < 0)
+threadData->fdout = -1;
+if (VIR_STRDUP(threadData->fdinname, path) < 0)
goto error;
-fd = fds[0];
And here we set 'fd' to be the pipe
} else {
-threadData->fdin = fds[0];
+threadData->fdin = -1;
threadData->fdout = fd;
-if (VIR_STRDUP(threadData->fdinname, "pipe") < 0 ||
-VIR_STRDUP(threadData->fdoutname, path) < 0)
+if (VIR_STRDUP(threadData->fdoutname, path) < 0)
goto error;
-fd = fds[1];
Likewise here
}
}
...now here 'fd' is passed to virFDStreamOpenInternal() and is the thing
that the event loop watches are registered against by virFDStreamAddCallback
With this change 'fd' is the actual plain file the thread is reading to/from,
so the callbacks are being registered against the plain file, not the pipe.
poll/select on POSIX always reports plain files as readable/writable even
when they would block. So with this change we're just going to busy loop
in the main event thread even when we'll block, which defeats the whole
purpose of having a iohelper and/or thread.
Oh, I've misunderstood what we've discussed on IRC then. The way I've
understood it was that if an FD is set to nonblock mode and poll()
claims there are some data available, subsequent read() might block. If
that was the case we would be safe with this code. However, I didn't
expect poll() to lie.
This code wouldn't be safe - anytime poll claims data available, we *must*
be able to read without blocking.
Any link for further reading on this? I guess it's not only us who has
to deal with this problem. Basically any application with poll() and
disk read()/write() has to suffer from this.
Yes, that's correct - QEMU has the same issue for example - it is why there
is no 'file:' protocol for migration for example - it would block the QEMU
main loop.
So what are our options here? Because I don't see any right now.
IIUC, you didn't want to use a pipe because you want to send structured
messages, not just plain data. If we just have a linked list of messages
there's nothing we can poll on, so we need to keep the pipe in use, but
find a way to get the special messages in the flow.
I think we could do a trick where we have two pipes in use, one for
monitoring the readability, and one for monitoring writability.
When the I/O thread has data on the queue ready for read by the main
thread, it can write a single byte to the read-monitor pipe.
When the I/O thread is ready to accept more data to write from the
main thread, it can write a single byte to the write-monitor pipe.
The main thread would monitor for POLLIN condition on both the
read-monitor pipe and write-monitor pipe.
Ah, indeed. This could work. But I also thought over different approach.
What I need really is transfer "you're in a data/hole X bytes long"
besides actual data. So I can use pipe for transferring the data as is
currently, and store the metadata into a structured message that would
the thread write/read and event loop read/write.
BTW, we also need to make sure the I/O thread doesn't proactively
queue too much data on the message queue when reading it, in case
the main thread is being slow
Re: [libvirt] [PATCH v2 1/4] qemu: refactor qemuDomainMachine* functions
On Tue, Apr 18, 2017 at 01:45:31PM +0200, Peter Krempa wrote: > On Tue, Apr 18, 2017 at 13:37:26 +0200, Pavel Hrdina wrote: > > Introduce new wrapper functions without *Machine* in the function > > name that take the whole virDomainDef structure as argument and > > call the existing functions with *Machine* in the function name. > > > > Change the arguments of existing functions to *machine* and *arch* > > because they don't need the whole virDomainDef structure and they > > could be used in places where we don't have virDomainDef. > > > > Signed-off-by: Pavel Hrdina > > --- > > > > New in v2: > > - add a wrappers that extract required data from virDomainDef structure > > This should have been split into the rename patch and then into addition > of the new wrappers. Calling this "refactor" is a convenient way to > avoid doing that. And I almost got away with it :). > > ACK regardless. Thanks signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] qemu: refactor qemuDomainMachine* functions
On Tue, Apr 18, 2017 at 13:37:26 +0200, Pavel Hrdina wrote: > Introduce new wrapper functions without *Machine* in the function > name that take the whole virDomainDef structure as argument and > call the existing functions with *Machine* in the function name. > > Change the arguments of existing functions to *machine* and *arch* > because they don't need the whole virDomainDef structure and they > could be used in places where we don't have virDomainDef. > > Signed-off-by: Pavel Hrdina > --- > > New in v2: > - add a wrappers that extract required data from virDomainDef structure This should have been split into the rename patch and then into addition of the new wrappers. Calling this "refactor" is a convenient way to avoid doing that. ACK regardless. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] qemu: refactor qemuDomainMachine* functions
Introduce new wrapper functions without *Machine* in the function
name that take the whole virDomainDef structure as argument and
call the existing functions with *Machine* in the function name.
Change the arguments of existing functions to *machine* and *arch*
because they don't need the whole virDomainDef structure and they
could be used in places where we don't have virDomainDef.
Signed-off-by: Pavel Hrdina
---
New in v2:
- add a wrappers that extract required data from virDomainDef structure
src/qemu/qemu_alias.c | 4 +-
src/qemu/qemu_capabilities.c | 10 ++--
src/qemu/qemu_command.c| 42 ++---
src/qemu/qemu_domain.c | 131 -
src/qemu/qemu_domain.h | 28 ++---
src/qemu/qemu_domain_address.c | 24
src/qemu/qemu_hotplug.c| 14 ++---
src/qemu/qemu_parse_command.c | 8 +--
8 files changed, 161 insertions(+), 100 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 05e1293ef0..914b2b94d8 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -152,14 +152,14 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
* first (and currently only) IDE controller is an integrated
* controller hardcoded with id "ide"
*/
-if (qemuDomainMachineHasBuiltinIDE(domainDef) &&
+if (qemuDomainHasBuiltinIDE(domainDef) &&
controller->idx == 0)
return VIR_STRDUP(controller->info.alias, "ide");
} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) {
/* for any Q35 machine, the first SATA controller is the
* integrated one, and it too is hardcoded with id "ide"
*/
-if (qemuDomainMachineIsQ35(domainDef) && controller->idx == 0)
+if (qemuDomainIsQ35(domainDef) && controller->idx == 0)
return VIR_STRDUP(controller->info.alias, "ide");
} else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) {
/* first USB device is "usb", others are normal "usb%d" */
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3ab99f450e..6d6bbf2467 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2315,7 +2315,7 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
/* If 'virt' supports PCI, it supports multibus.
* No extra conditions here for simplicity.
*/
-if (qemuDomainMachineIsVirt(def))
+if (qemuDomainIsVirt(def))
return true;
return false;
@@ -5369,7 +5369,7 @@ virQEMUCapsSupportsChardev(const virDomainDef *def,
return false;
if ((def->os.arch == VIR_ARCH_PPC) || ARCH_IS_PPC64(def->os.arch)) {
-if (!qemuDomainMachineIsPSeries(def))
+if (!qemuDomainIsPSeries(def))
return false;
/* only pseries need -device spapr-vty with -chardev */
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
@@ -5396,8 +5396,8 @@ virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT))
return false;
-return qemuDomainMachineIsI440FX(def) ||
-qemuDomainMachineIsQ35(def) ||
+return qemuDomainIsI440FX(def) ||
+qemuDomainIsQ35(def) ||
STREQ(def->os.machine, "isapc");
}
@@ -5409,7 +5409,7 @@ virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps,
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT))
return false;
-return qemuDomainMachineIsQ35(def);
+return qemuDomainIsQ35(def);
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 57654246c0..3d64e3cc4d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1390,7 +1390,7 @@ qemuCheckCCWS390AddressSupport(const virDomainDef *def,
const char *devicename)
{
if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-if (!qemuDomainMachineIsS390CCW(def)) {
+if (!qemuDomainIsS390CCW(def)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("cannot use CCW address type for device "
"'%s' using machine type '%s'"),
@@ -2294,7 +2294,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
/* PowerPC pseries based VMs do not support floppy device */
if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
-qemuDomainMachineIsPSeries(def)) {
+qemuDomainIsPSeries(def)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("PowerPC pseries machines do not support floppy
device"));
return -1;
@@ -2345,7 +2345,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
disk->info.alias) < 0)
return -1;
-if (!qemuDomainMachineNeedsFDC(def)) {
+if (!qemuDomainNeedsFDC(def)) {
virCo
Re: [libvirt] server side copy from one volume to another
On 04/18/2017 01:03 PM, Vasiliy Tolstov wrote: 2017-04-18 8:39 GMT+03:00 Michal Privoznik : s that right? Here I'm successfully cloning a volume across two storage pools: Yes, it works but Source image: image: /var/lib/libvirt/images/sda/143177 file format: qcow2 virtual size: 5.0G (5368709120 bytes) disk size: 902M cluster_size: 65536 Format specific information: compat: 0.10 refcount bits: 16 Dest image: image: /var/lib/libvirt/images/sda/test file format: qcow2 virtual size: 5.0G (5368709120 bytes) disk size: 902M cluster_size: 65536 Format specific information: compat: 0.10 refcount bits: 16 But -rw--- 1 root 908M Apr 18 13:59 143177 -rw--- 1 root 902M Apr 18 13:59 test Why size is changed on destination image? In case of cp --spwarse=always size does not changed. That's because libvirt does not use `cp` to copy the disk images. We use qemu-img for copying. And we do not guarantee that the disk structure does not change. We guarantee that the data stored on the disk doesn't change. Otherwise we wouldn't be able to convert disk images. NB, with sparse files - it's not as straightforward either. Some filesystems may decide that the hole is not big enough so they do allocate it actually. XFS is a very good example of such behaviour. So the data/hole structure of a file may not line up with the structure of a copy. Although, the data as read from the files are the same. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] server side copy from one volume to another
2017-04-18 14:03 GMT+03:00 Vasiliy Tolstov : > Why size is changed on destination image? In case of cp > --spwarse=always size does not changed. This is qemu-img feature... Does libvirt garantie that successful return from copy volume function not breaks dst volume? (md5 can't be checked because file is changed) -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] server side copy from one volume to another
2017-04-18 8:39 GMT+03:00 Michal Privoznik : > s that right? Here I'm successfully cloning a volume across two storage pools: Yes, it works but Source image: image: /var/lib/libvirt/images/sda/143177 file format: qcow2 virtual size: 5.0G (5368709120 bytes) disk size: 902M cluster_size: 65536 Format specific information: compat: 0.10 refcount bits: 16 Dest image: image: /var/lib/libvirt/images/sda/test file format: qcow2 virtual size: 5.0G (5368709120 bytes) disk size: 902M cluster_size: 65536 Format specific information: compat: 0.10 refcount bits: 16 But -rw--- 1 root 908M Apr 18 13:59 143177 -rw--- 1 root 902M Apr 18 13:59 test Why size is changed on destination image? In case of cp --spwarse=always size does not changed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
On Tue, Apr 18, 2017 at 12:08:30PM +0200, Peter Krempa wrote: > On Tue, Apr 18, 2017 at 12:00:12 +0200, Pavel Hrdina wrote: > > On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote: > > > On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote: > > > > > > Was this the problem that virt-manager allowed to use the IDE bus on > > > Q35? > > > > Yes :) > > Then please add something like "Since virt-manager (virt-install?) uses > this data it would allow to create configuration which would be rejected > by libvirt." or something like that ... I don't see any value in mentioning virt-manager in commit message, the domain capability output is wrong regardless of who uses it. > > > > > > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964 > > Since this BZ does not clearly state that fact. > > > > > > > > > Signed-off-by: Pavel Hrdina > > > > --- > > > > src/qemu/qemu_capabilities.c | 4 +++- > > > > tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - > > > > tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - > > > > tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml| 1 - > > > > tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml| 1 - > > > > tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - > > > > tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - > > > > 7 files changed, 3 insertions(+), 7 deletions(-) > > > > > > ACK, > > > > > > but I think you should follow up and do the same for the floppy device > > > which AFAIK does not exist on aarch64 and others. There was an attempt > > > to do this but only for ppc in 020a178318 > > > > There is a lot of space for improvement, I'll add it to my todo list :) > > Yes. There is. As in every other part of libvirt :) signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
On Tue, Apr 18, 2017 at 12:00:12 +0200, Pavel Hrdina wrote: > On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote: > > On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote: > > > > Was this the problem that virt-manager allowed to use the IDE bus on > > Q35? > > Yes :) Then please add something like "Since virt-manager (virt-install?) uses this data it would allow to create configuration which would be rejected by libvirt." or something like that ... > > > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964 Since this BZ does not clearly state that fact. > > > > > > Signed-off-by: Pavel Hrdina > > > --- > > > src/qemu/qemu_capabilities.c | 4 +++- > > > tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - > > > tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - > > > tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml| 1 - > > > tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml| 1 - > > > tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - > > > tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - > > > 7 files changed, 3 insertions(+), 7 deletions(-) > > > > ACK, > > > > but I think you should follow up and do the same for the floppy device > > which AFAIK does not exist on aarch64 and others. There was an attempt > > to do this but only for ppc in 020a178318 > > There is a lot of space for improvement, I'll add it to my todo list :) Yes. There is. As in every other part of libvirt :) signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote: > On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote: > > Was this the problem that virt-manager allowed to use the IDE bus on > Q35? Yes :) > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964 > > > > Signed-off-by: Pavel Hrdina > > --- > > src/qemu/qemu_capabilities.c | 4 +++- > > tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - > > tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - > > tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml| 1 - > > tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml| 1 - > > tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - > > tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - > > 7 files changed, 3 insertions(+), 7 deletions(-) > > ACK, > > but I think you should follow up and do the same for the floppy device > which AFAIK does not exist on aarch64 and others. There was an attempt > to do this but only for ppc in 020a178318 There is a lot of space for improvement, I'll add it to my todo list :) Pavel signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Add mdev reporting capability to the nodedev driver
On Tue, Apr 18, 2017 at 09:52:38AM +0100, Daniel P. Berrange wrote: > On Tue, Apr 18, 2017 at 10:49:40AM +0200, Martin Polednik wrote: > > On 12/04/17 16:19 -0600, Alex Williamson wrote: > > > On Wed, 5 Apr 2017 09:21:17 +0100 > > > "Daniel P. Berrange" wrote: > > > > > > > On Wed, Apr 05, 2017 at 08:12:31AM +0200, Erik Skultety wrote: > > > > > On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote: > > > > > > On Wed, Mar 29, 2017 at 02:51:10PM +0200, Erik Skultety wrote: > > > > > > > This series enables the node device driver to report information > > > > > > > about the > > > > > > > existing mediated devices on the host. There is no device > > > > > > > creation involved > > > > > > > yet. The information reported by the node device driver is split > > > > > > > into two > > > > > > > parts, one that is reported within the physical parent's > > > > > > > capabilities > > > > > > > (the generic stuff that comes from the mdev types' sysfs > > > > > > > attributes, note the > > > > > > > 'description' attribute which is verbatim - raw,unstructured > > > > > > > string) and the > > > > > > > other that is reported within the mdev child device and merely > > > > > > > contains the > > > > > > > mdev type id, which the device was instantiated from, and the > > > > > > > iommu group > > > > > > > number. > > > > > > > > > > > > > > Basically, the format of the XML I went for is as follows: > > > > > > > > > > > > > > PCI parent: > > > > > > > > > > > > > > pci__06_00_0 > > > > > > > /sys/devices/.../:06:00.0 > > > > > > > pci__05_08_0 > > > > > > > ... > > > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > > > > GRID M60-0B > > > > > > > num_heads=2, frl_config=45, > > > > > > > framebuffer=512M, max_resolution=2560x1600, > > > > > > > max_instance=16 > > > > > > > > > > > > This 'description' field is pretty horrific. > > > > > > > > > > > > We were quite clear that we were not going to expose arbitrary > > > > > > attributes > > > > > > in the XML without modelling them explicitly as XML elements. Using > > > > > > the > > > > > > description field in this way is just doing arbitrary attribute > > > > > > passthrough > > > > > > via the backdoor - it is clear that applications are doing to end > > > > > > up parsing > > > > > > this 'description' data and will them complain if we later change > > > > > > it. > > > > > > > > > > > > > > > > I remember us stating that, but this is the case with NVIDIA (who at > > > > > least > > > > > reports some useful information in it) and Intel - what if other > > > > > vendor comes > > > > > and creates a valid, human readable description of a device without > > > > > specifying > > > > > any attributes like in the case above? Just because some vendors > > > > > "abused" the > > > > > attribute doesn't mean we should stop reporting it completely. IMHO > > > > > the name > > > > > "description" should mean that it's something raw, possibly > > > > > unstructured, and > > > > > thus the applications should treat it that way. Now, this is my bad > > > > > and I need > > > > > to revisit the last patch with documentation and add a paragraph for > > > > > the > > > > > element as an optional element with raw data. > > > > > > > > > > Until all the attributes are properly unified/standardized under > > > > > sysfs ABI and > > > > > respected by vendors, I think we should report everything we're able > > > > > to gather > > > > > about a device, description included. If properly documented, nobody > > > > > can > > > > > complain about us breaking anything, since how do you guarantee > > > > > format-less raw > > > > > string anyway? After all, applications will end up parsing it anyway, > > > > > be it from > > > > > our XML or not. > > > > > > > > I understand your point, but I'm still not in favour of exposing this > > > > info > > > > because it is a clear cut attempt to do arbitrary attribute passthrough > > > > to > > > > libvirt. > > > > > > > > All the attributes show there can be determined by a simple lookup > > > > based on > > > > the name field "GRID M60-0B". So if apps want to know the number of > > > > heads, > > > > framebuffer size, etc, etc I think they should just maintain a lookup > > > > map > > > > based on name in their own code, until we explicitly model this stuff in > > > > the XML. > > > > > > > > Once we model the attributes in the XML, this description adds no useful > > > > info that we wouldn't already be reported, and before we model it in the > > > > XML, this is just clearly an abuse of our design statement that we are > > > > not > > > > doing generic attribute passthrough. > > > > > > I told Erik I'd jump in here, but I think I might agree with Dan. On > > > the kernel side, the description attribute was an attempt to pull > > > ourselves out of a rat hole of trying to figure out how to classify > > > devices and then figu
Re: [libvirt] [PATCH 1/4] qemu: refactor qemuDomainMachineIs* and qemuDomainMachineNeedsFDC
On Tue, Apr 18, 2017 at 11:38:09AM +0200, Peter Krempa wrote: > On Thu, Apr 13, 2017 at 16:57:13 +0200, Pavel Hrdina wrote: > > These functions don't require the whole virDomainDef structure, > > they only need *machine* and *arch*. > > > > Signed-off-by: Pavel Hrdina > > --- > > src/qemu/qemu_alias.c | 4 +-- > > src/qemu/qemu_capabilities.c | 10 +++--- > > src/qemu/qemu_command.c| 42 +++--- > > src/qemu/qemu_domain.c | 79 > > ++ > > src/qemu/qemu_domain.h | 14 > > src/qemu/qemu_domain_address.c | 16 - > > src/qemu/qemu_hotplug.c| 14 > > src/qemu/qemu_parse_command.c | 8 ++--- > > 8 files changed, 95 insertions(+), 92 deletions(-) > > In most cases where this is used you have the domain object so it's more > straightfowrward to extract it from that. > > I'm suggesting that you modify qemuDomainMachineHasBuiltinIDE and > friends to be a wrapper which only extracts the os.machine object from > def and calls a new helper which will not take the domain (and not > contain the word "Domain"). That way you won't have to make all other > callers uglier. I had the same idea to create wrapper just for this case and don't mess with the existing functions. I agree that having nice helper that will extract required variables form virDomainDef is convenient. I'll rework this patch. Thanks, Pavel > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/5] Prevent losing IPv6 routes due to forwarding
Yalan 你好 On Mon, 2017-04-17 at 17:30 +0800, Yalan Zhang wrote: > I have tested it, it works well. But the interface name will repeat 2 times. > Please help to confirm this, and if below test for a single port host is > enough? > > # cat /proc/sys/net/ipv6/conf/enp0s25/accept_ra > 1 > > enable network default with ipv6 ip section > > # virsh net-start default > error: Failed to start network default > error: internal error: Check the host setup: enabling IPv6 forwarding with RA > routes without accept_ra set to 2 is > likely to cause routes loss. Interfaces to look at: enp0s25, enp0s25 Just to help me confirm my intuition: do you have several RA routes defined for the same device? > # echo 2 > /proc/sys/net/ipv6/conf/enp0s25/accept_ra > > # virsh net-start default > Network default started > > try create: > > # virsh net-create default.xml > error: Failed to create network from default.xml > error: internal error: Check the host setup: enabling IPv6 forwarding with RA > routes without accept_ra set to 2 is > likely to cause routes loss. Interfaces to look at: enp0s25, enp0s25 This one sounds weird: if the accept_ra is set to 2 as you report you did, you shouldn't get that error. -- Cedric > On Wed, Mar 15, 2017 at 10:45 PM, Cédric Bosdonnat > wrote: > > Hi Laine, all, > > > > Here is the v2 of my series. The changes are: > > > > * Add a commit to create a virNetDevGetName() function > > * Fix Laine's comments > > > > Cédric Bosdonnat (5): > > util: extract the request sending code from virNetlinkCommand() > > util: add virNetlinkDumpCommand() > > bridge_driver.c: more uses of SYSCTL_PATH > > util: add virNetDevGetName() function > > network: check accept_ra before enabling ipv6 forwarding > > > > src/libvirt_private.syms | 3 + > > src/network/bridge_driver.c | 25 --- > > src/util/virnetdev.c | 19 ++ > > src/util/virnetdev.h | 2 + > > src/util/virnetdevip.c | 158 > > > > src/util/virnetdevip.h | 1 + > > src/util/virnetlink.c | 145 ++-- > > src/util/virnetlink.h | 9 +++ > > 8 files changed, 319 insertions(+), 43 deletions(-) > > > > -- > > 2.11.0 > > > > -- > > 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] [PATCH 1/4] qemu: refactor qemuDomainMachineIs* and qemuDomainMachineNeedsFDC
On Thu, Apr 13, 2017 at 16:57:13 +0200, Pavel Hrdina wrote: > These functions don't require the whole virDomainDef structure, > they only need *machine* and *arch*. > > Signed-off-by: Pavel Hrdina > --- > src/qemu/qemu_alias.c | 4 +-- > src/qemu/qemu_capabilities.c | 10 +++--- > src/qemu/qemu_command.c| 42 +++--- > src/qemu/qemu_domain.c | 79 > ++ > src/qemu/qemu_domain.h | 14 > src/qemu/qemu_domain_address.c | 16 - > src/qemu/qemu_hotplug.c| 14 > src/qemu/qemu_parse_command.c | 8 ++--- > 8 files changed, 95 insertions(+), 92 deletions(-) In most cases where this is used you have the domain object so it's more straightfowrward to extract it from that. I'm suggesting that you modify qemuDomainMachineHasBuiltinIDE and friends to be a wrapper which only extracts the os.machine object from def and calls a new helper which will not take the domain (and not contain the word "Domain"). That way you won't have to make all other callers uglier. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] tests: domaincapstest: add test for Q35 machine type
On Thu, Apr 13, 2017 at 16:57:16 +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > .../domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 125 > + > tests/domaincapstest.c | 4 + > 2 files changed, 129 insertions(+) > create mode 100644 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml > > diff --git a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml > b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml > new file mode 100644 > index 00..f6d54d9a12 > --- /dev/null > +++ b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml > @@ -0,0 +1,125 @@ > + [...] > + [...] > + [...] > + Haswell > + Haswell-noTSX > + Conroe > + Broadwell > + Broadwell-noTSX > + 486 Lol. Really? A Q35 machine with 486 CPU? > + ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] qemu: report IDE bus in domain capabilities only if it's supported
On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote: Was this the problem that virt-manager allowed to use the IDE bus on Q35? > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964 > > Signed-off-by: Pavel Hrdina > --- > src/qemu/qemu_capabilities.c | 4 +++- > tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - > tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - > tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml| 1 - > tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml| 1 - > tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - > tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - > 7 files changed, 3 insertions(+), 7 deletions(-) ACK, but I think you should follow up and do the same for the floppy device which AFAIK does not exist on aarch64 and others. There was an attempt to do this but only for ppc in 020a178318 Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] qemu: use qemuDomainMachineIsPSeries
On Thu, Apr 13, 2017 at 16:57:14 +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina > --- > src/qemu/qemu_capabilities.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V3] Expose resource control capabilites on cache bank
On Fri, Apr 07, 2017 at 01:10:38PM +0100, Daniel P. Berrange wrote: On Fri, Apr 07, 2017 at 08:07:23PM +0800, Eli Qiao wrote: On Friday, 7 April 2017 at 7:06 PM, Daniel P. Berrange wrote: > On Fri, Apr 07, 2017 at 01:04:57PM +0200, Martin Kletzander wrote: > > On Fri, Apr 07, 2017 at 05:47:54PM +0800, Eli Qiao wrote: > > > > > > > > The name doesn't really matter that much, 'scope' makes a bit more > > > > sense, 'type' is consistent with the cache bank specification, I'm fine > > > > with any. The big question here was if it is possible to have: > > > > > > > > > > > > > > > > > > > > > > > > > > > > And from what you say, the simple answer is "yes". So we need to have > > > > the attribute there in the control element as well. > > > > > > > > > > > > > > > > Dan/Martin > > > > > > Could you please advice which should be changed ? LoL > > > > > > This is the output if I enabled CDP > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. change nallocations to allocations/max_allocation? > > > > Dan said he's fine with both, I'd probably go for max_allocations > > > > > 2. change type to scope ? > > > > I don't care, pros for both in the previous mail. > > Both attributes take the same enum values, so it is best to be consistent > with the attribute name. > Thanks very much for Daniel & Martin Forgive me to ping you again, just make sure I am in correct place : If I read your comments correctly, we can go with: @Daniel, the enum values are same with `type` unified: 0 instruction: 1 data: 2 but scope should be both(0)/code(1)/data(2), so the attribute name will be 'both' and 'unified' mean the same thing. 'instruction' and 'code' mean the same thing to. So we should use the same terminology for both attributes. IOW, I suggest we use 'both', 'code', and 'data' everywhere. In that case, please also tell me if I should use 'type' or 'scope' for the cache info. I, personally, have never head anyone referring to a "code cache" or "both cache", but I'm fine with anything as long as we're consistent. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Add mdev reporting capability to the nodedev driver
On Tue, Apr 18, 2017 at 10:49:40AM +0200, Martin Polednik wrote: > On 12/04/17 16:19 -0600, Alex Williamson wrote: > > On Wed, 5 Apr 2017 09:21:17 +0100 > > "Daniel P. Berrange" wrote: > > > > > On Wed, Apr 05, 2017 at 08:12:31AM +0200, Erik Skultety wrote: > > > > On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote: > > > > > On Wed, Mar 29, 2017 at 02:51:10PM +0200, Erik Skultety wrote: > > > > > > This series enables the node device driver to report information > > > > > > about the > > > > > > existing mediated devices on the host. There is no device creation > > > > > > involved > > > > > > yet. The information reported by the node device driver is split > > > > > > into two > > > > > > parts, one that is reported within the physical parent's > > > > > > capabilities > > > > > > (the generic stuff that comes from the mdev types' sysfs > > > > > > attributes, note the > > > > > > 'description' attribute which is verbatim - raw,unstructured > > > > > > string) and the > > > > > > other that is reported within the mdev child device and merely > > > > > > contains the > > > > > > mdev type id, which the device was instantiated from, and the iommu > > > > > > group > > > > > > number. > > > > > > > > > > > > Basically, the format of the XML I went for is as follows: > > > > > > > > > > > > PCI parent: > > > > > > > > > > > > pci__06_00_0 > > > > > > /sys/devices/.../:06:00.0 > > > > > > pci__05_08_0 > > > > > > ... > > > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > GRID M60-0B > > > > > > num_heads=2, frl_config=45, framebuffer=512M, > > > > > > max_resolution=2560x1600, max_instance=16 > > > > > > > > > > This 'description' field is pretty horrific. > > > > > > > > > > We were quite clear that we were not going to expose arbitrary > > > > > attributes > > > > > in the XML without modelling them explicitly as XML elements. Using > > > > > the > > > > > description field in this way is just doing arbitrary attribute > > > > > passthrough > > > > > via the backdoor - it is clear that applications are doing to end up > > > > > parsing > > > > > this 'description' data and will them complain if we later change it. > > > > > > > > > > > > > I remember us stating that, but this is the case with NVIDIA (who at > > > > least > > > > reports some useful information in it) and Intel - what if other vendor > > > > comes > > > > and creates a valid, human readable description of a device without > > > > specifying > > > > any attributes like in the case above? Just because some vendors > > > > "abused" the > > > > attribute doesn't mean we should stop reporting it completely. IMHO the > > > > name > > > > "description" should mean that it's something raw, possibly > > > > unstructured, and > > > > thus the applications should treat it that way. Now, this is my bad and > > > > I need > > > > to revisit the last patch with documentation and add a paragraph for the > > > > element as an optional element with raw data. > > > > > > > > Until all the attributes are properly unified/standardized under sysfs > > > > ABI and > > > > respected by vendors, I think we should report everything we're able to > > > > gather > > > > about a device, description included. If properly documented, nobody can > > > > complain about us breaking anything, since how do you guarantee > > > > format-less raw > > > > string anyway? After all, applications will end up parsing it anyway, > > > > be it from > > > > our XML or not. > > > > > > I understand your point, but I'm still not in favour of exposing this info > > > because it is a clear cut attempt to do arbitrary attribute passthrough to > > > libvirt. > > > > > > All the attributes show there can be determined by a simple lookup based > > > on > > > the name field "GRID M60-0B". So if apps want to know the number of heads, > > > framebuffer size, etc, etc I think they should just maintain a lookup map > > > based on name in their own code, until we explicitly model this stuff in > > > the XML. > > > > > > Once we model the attributes in the XML, this description adds no useful > > > info that we wouldn't already be reported, and before we model it in the > > > XML, this is just clearly an abuse of our design statement that we are not > > > doing generic attribute passthrough. > > > > I told Erik I'd jump in here, but I think I might agree with Dan. On > > the kernel side, the description attribute was an attempt to pull > > ourselves out of a rat hole of trying to figure out how to classify > > devices and then figure out what attributes for each class might be > > common across vendors. Clearly it'd be great to somehow know that a > > device is a GPU and has a resolution and graphics memory attribute, but > > getting there is hard. A free-form description field is sort of a > > catch-all where the vendor can provide a stop-gap and it's useful for > > human consumption.
Re: [libvirt] Add mdev reporting capability to the nodedev driver
On 12/04/17 16:19 -0600, Alex Williamson wrote: On Wed, 5 Apr 2017 09:21:17 +0100 "Daniel P. Berrange" wrote: On Wed, Apr 05, 2017 at 08:12:31AM +0200, Erik Skultety wrote: > On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote: > > On Wed, Mar 29, 2017 at 02:51:10PM +0200, Erik Skultety wrote: > > > This series enables the node device driver to report information about the > > > existing mediated devices on the host. There is no device creation involved > > > yet. The information reported by the node device driver is split into two > > > parts, one that is reported within the physical parent's capabilities > > > (the generic stuff that comes from the mdev types' sysfs attributes, note the > > > 'description' attribute which is verbatim - raw,unstructured string) and the > > > other that is reported within the mdev child device and merely contains the > > > mdev type id, which the device was instantiated from, and the iommu group > > > number. > > > > > > Basically, the format of the XML I went for is as follows: > > > > > > PCI parent: > > > > > > pci__06_00_0 > > > /sys/devices/.../:06:00.0 > > > pci__05_08_0 > > > ... > > > > > > ... > > > > > > > > > GRID M60-0B > > > num_heads=2, frl_config=45, framebuffer=512M, max_resolution=2560x1600, max_instance=16 > > > > This 'description' field is pretty horrific. > > > > We were quite clear that we were not going to expose arbitrary attributes > > in the XML without modelling them explicitly as XML elements. Using the > > description field in this way is just doing arbitrary attribute passthrough > > via the backdoor - it is clear that applications are doing to end up parsing > > this 'description' data and will them complain if we later change it. > > > > I remember us stating that, but this is the case with NVIDIA (who at least > reports some useful information in it) and Intel - what if other vendor comes > and creates a valid, human readable description of a device without specifying > any attributes like in the case above? Just because some vendors "abused" the > attribute doesn't mean we should stop reporting it completely. IMHO the name > "description" should mean that it's something raw, possibly unstructured, and > thus the applications should treat it that way. Now, this is my bad and I need > to revisit the last patch with documentation and add a paragraph for the > element as an optional element with raw data. > > Until all the attributes are properly unified/standardized under sysfs ABI and > respected by vendors, I think we should report everything we're able to gather > about a device, description included. If properly documented, nobody can > complain about us breaking anything, since how do you guarantee format-less raw > string anyway? After all, applications will end up parsing it anyway, be it from > our XML or not. I understand your point, but I'm still not in favour of exposing this info because it is a clear cut attempt to do arbitrary attribute passthrough to libvirt. All the attributes show there can be determined by a simple lookup based on the name field "GRID M60-0B". So if apps want to know the number of heads, framebuffer size, etc, etc I think they should just maintain a lookup map based on name in their own code, until we explicitly model this stuff in the XML. Once we model the attributes in the XML, this description adds no useful info that we wouldn't already be reported, and before we model it in the XML, this is just clearly an abuse of our design statement that we are not doing generic attribute passthrough. I told Erik I'd jump in here, but I think I might agree with Dan. On the kernel side, the description attribute was an attempt to pull ourselves out of a rat hole of trying to figure out how to classify devices and then figure out what attributes for each class might be common across vendors. Clearly it'd be great to somehow know that a device is a GPU and has a resolution and graphics memory attribute, but getting there is hard. A free-form description field is sort of a catch-all where the vendor can provide a stop-gap and it's useful for human consumption. It would be inadvisable to parse it with machine code though. Including it in the XML sort of invites that possibility. A given mdev type is supposed to be stable. It should always point to a software equivalent device, including capabilities and resources. It should therefore be valid for upper level software to use the type to lookup from a human generated table the properties for that device. Of From an upper level software's perspective, this is super inefficient unless database like pci IDs exist. Each management software maintaining their possibly outdated lookup table may lead to inconsistence in the user experience and data. If such database existed, there is no reason for libvirt not to use it and report the data in some sane format. course there's ab
Re: [libvirt] Trouble with virsh on Windows
On Mon, Apr 17, 2017 at 09:49:01AM -0400, Charles Bancroft wrote: > Hi List, > > I have a question about some troubles I am having with virsh on Windows > x64. I am currently running a KVM server on a linux box and need to > allow Windows clients to access it. I have set up the server for access > with: > > ``` > listen_tls = 0 > listen_tcp = 1 > auth_tcp = "none" > ``` I assume you're merely doing this for sake of testing / debugging. This opens libvirtd connections with no access control, which is equivalent to running an SSH server allowing root logins with no password. ie you never want to do this in real deployments. > to allow for remote access while testing things. I have verified that I > can connect remotely from other linux machines, but my windows machine > always reports: > > ``` > error: failed to connect to the hypervisor > error: An error occurred, but the cause is unknown > ``` > > On the server side I see: > ``` > 10435: error : virNetSocketReadWire:1793 : Cannot recv data: Connection > reset by peer > ``` > > This happens no matter what build of virsh for windows I am using. I > have tried 3 different mingw-64 builds and versions and am about to just > build my own from scratch. I have verified that it is not the firewall > or the version of Windows I am using. I am also able to use virsh > within a docker or from the Windows Subsystem for Linux bash prompt. > > Any ideas as to what could be causing this connection reset? It happens > as soon as a connection attempt occurs and I am running out of idea. When running virsh, set 'LIBVIRT_DEBUG=1' environment variable, so we can see some debug logs, which might give a hint as to what is wrong. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Use-after-free fix and cleanups
On Mon, Apr 10, 2017 at 02:52 PM +0200, Michal Privoznik wrote: > On 04/03/2017 10:24 AM, Marc Hartmayer wrote: >> While looking at a use-after-free situation going through how the QEMU >> monitor is set up I noticed some things. These cleanups and the fix >> for the use-after-free are the result of that. >> >> Marc Hartmayer (5): >> qemu: Fix two use-after-free situations >> qemu: Turn qemuDomainLogContext into virObject >> qemu: Implement qemuMonitorRegister() >> qemu: remove ATTRIBUTE_UNUSED in qemuProcessHandleMonitorEOF >> refactoring: Use the return value of virObjectRef directly >> >> src/datatypes.c | 6 ++-- >> src/qemu/qemu_domain.c | 72 -- >> src/qemu/qemu_domain.h | 2 -- >> src/qemu/qemu_monitor.c | 82 >> ++-- >> src/qemu/qemu_monitor.h | 6 >> src/qemu/qemu_process.c | 12 +++ >> src/rpc/virnetclientstream.c | 4 +-- >> src/rpc/virnetserver.c | 9 ++--- >> tests/qemumonitortestutils.c | 3 +- >> 9 files changed, 121 insertions(+), 75 deletions(-) >> > > ACKed and pushed. Thanks. Thanks. > > Michal > -- 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 v2] hyperv: recognize array property as distinct type.
2017-04-11 21:42 GMT+02:00 Dawid Zamirski :
> When hyperv code generator for WMI classes identifies common
> properties, it needs to take into account array type as a distinct
> type, i.e string != string[]. This is the case where v1 of the
> Msvm_VirtualSystemSettingData has Notes property as string whereas v2
> uses Notes[], therefore they have to be treated as different fields and
> cannot be placed in the "common" struct.
> ---
>
> changes in v2:
> * move notes variable assignment outside of the loop so that it's not
>reset at each iteration.
>
> src/hyperv/hyperv_driver.c | 25 +++--
> src/hyperv/hyperv_wmi_generator.py | 2 +-
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 090ea24..350f123 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -894,8 +894,29 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int
> flags)
> if (VIR_STRDUP(def->name, computerSystem->data.common->ElementName) < 0)
> goto cleanup;
>
> -if (VIR_STRDUP(def->description,
> virtualSystemSettingData->data.common->Notes) < 0)
> -goto cleanup;
> +if (priv->wmiVersion == HYPERV_WMI_VERSION_V1) {
> +if (VIR_STRDUP(def->description,
> + virtualSystemSettingData->data.v1->Notes) < 0)
> +goto cleanup;
> +} else if (priv->wmiVersion == HYPERV_WMI_VERSION_V2 &&
> + virtualSystemSettingData->data.v2->Notes.data != NULL) {
> +char **notes = (char **)
> virtualSystemSettingData->data.v2->Notes.data;
> +virBuffer buf = VIR_BUFFER_INITIALIZER;
> +size_t i = 0;
> +
> +/* in practice Notes has 1 element */
> +for (i = 0; i < virtualSystemSettingData->data.v2->Notes.count; i++)
> {
> +/* but if there's more than 1, separate by double new line */
> +if (virBufferUse(&buf) > 0)
> +virBufferAddLit(&buf, "\n\n");
> +
> +virBufferAdd(&buf, *notes, -1);
> +notes++;
> +}
virBufferContentAndReset will silently return NULL if an error (e.g.
OOM) occurred while building the buffer content. You current code will
silently set description to NULL instead of properly reporting an
error. Use virBufferCheckError as shown here:
http://libvirt.org/hacking.html#strbuf
virBufferCheckError will report and error for you if there is one.
if (virBufferCheckError(&buf))
goto cleanup;
> +
> +def->description = virBufferContentAndReset(&buf);
> +}
> +
--
Matthias Bolte
http://photron.blogspot.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] xenconfig: avoid double free on OOM testing
On Thu, Apr 13, 2017 at 11:07:41 -0600, Jim Fehlig wrote: > Fix xlconfig channel tests when OOM testing is enabled. > > TEST: xlconfigtest > 32) Xen XL-2-XML Format channel-unix ... OK > Test OOM for nalloc=55 > *** Error in > `/home/jfehlig/virt/upstream/libvirt/tests/.libs/xlconfigtest': double free > or corruption (fasttop): 0x00679550 *** > ... > (gdb) bt > #0 0x736875af in raise () from /lib64/libc.so.6 > #1 0x736889aa in abort () from /lib64/libc.so.6 > #2 0x736c5150 in __libc_message () from /lib64/libc.so.6 > #3 0x736cb4f6 in malloc_printerr () from /lib64/libc.so.6 > #4 0x736cbcee in _int_free () from /lib64/libc.so.6 > #5 0x7782babf in virFree (ptrptr=0x7fffdca8) at > util/viralloc.c:582 > #6 0x0042f2f3 in xenParseXLChannel (conf=0x677350, def=0x6815b0) at > xenconfig/xen_xl.c:788 > #7 0x0042f44e in xenParseXL (conf=0x677350, caps=0x6832b0, > xmlopt=0x67f6e0) at xenconfig/xen_xl.c:828 > #8 0x004105a3 in testCompareFormatXML ( > xlcfg=0x6811e0 > "/home/jfehlig/virt/upstream/libvirt/tests/xlconfigdata/test-channel-unix.cfg", > xml=0x681110 > "/home/jfehlig/virt/upstream/libvirt/tests/xlconfigdata/test-channel-unix.xml", > replaceVars=false) > at xlconfigtest.c:152 > > When a channel is successfully parsed and its path and name fields > assigned from local variables, set the local variables to NULL to > prevent a double free on error. > > Signed-off-by: Jim Fehlig > --- > src/xenconfig/xen_xl.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) ACK signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
