[libvirt] [PATCH] tests: fix test segfault when libxl configuration setup fails.
This commit fixes a segmentation fault caused by missing conditional to check if libxl configuration was properly created by the test. If the configuration was not properly created, libxlDriverConfigNew() function will return NULL and cause a segfault at cfg->caps = NULL during the cleanup. Signed-off-by: Julio Faracco --- tests/libxlxml2domconfigtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 34a63e22b5..a9758c40cb 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -138,7 +138,8 @@ testCompareXMLToDomConfig(const char *xmlfile, libxl_domain_config_dispose(); libxl_domain_config_dispose(); xtl_logger_destroy(log); -cfg->caps = NULL; +if (cfg) +cfg->caps = NULL; virObjectUnref(cfg); return ret; } -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of libvirt-4.6.0 Release Candidate 2
On 08/01/2018 04:44 PM, Laine Stump wrote:
> (Let's see now which people were the ones who believe it's a huge
> insult to Cc them in list replies, and which are the ones who appreciate
> the Cc and use it as a flag to raise the visibility of the message in
> their mail client . ?. Ah, I give up, just leaving in the Cc's
> and let the complaints fly)(For the record, I like explicit Cc's when
> you want to make sure I see something - that sends a copy to my phone,
> which can be mildly annoying if it's a series of 40 patches, but also
> very effective in getting my attention :-P)
>
> On 08/01/2018 06:40 AM, Michal Privoznik wrote:
>> On 07/31/2018 07:19 PM, Ján Tomko wrote:
>>> And I'm still unsure about leaving in
>>> commit 55ce65646348884656fd7bf3f109ebf8f7603494
>>> qemu: Use the correct vm def on cold attach
>>> https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=55ce6564634
>>> Which means attach-device --live --config will attach an interface
>>> with a different MAC address in live and persistent definition. Laine?
>
> (Let's see, you said my name once in the Cc, and once here. You really
> need to say it *three* times to get my attention ala Beetlejuice or
> Biggy Smalls. (and it helps to recite it into a mirror in a dark room,
> apparently)).
>
>>>
>> Well, It's not only MAC address that can change. The device address
>> might change too. This points to a broader problem. When we are parsing
>> a device XML we fill in the blanks in postParse callbacks. However,
>> those look only at either live or at inactive XML. Not at both at the
>> same time. So how can we fill in the blanks that would be valid for both
>> XMLs?
>
> Yeah, this has been a problem for a long time (it's also problematic
> for, e.g. the index of PCI controllers, which can cascade into
> differences in the PCI addresses of devices that are connected to those
> controllers; similar problem for other types of controllers of course).
> I think I even tried fixing it once, but the solution was inadequate.
>
> The problem comes when someone uses a mixture of --live only, --config
> only, and --live+--config device attaches/detaches. At the base of the
> problem is the fact that the live and config domaindef objects are
> stored completely independent of each other, and the code that assigns
> PCI addresses "seeds" the list of in-use address from a single domaindef
> object. Perhaps we need to merge these into a single object where each
> device is marked as "live", "config" or "both" - then any new device
> added would be guaranteed to not conflict with any existing device in
> live or config - the unified device list would be used to determine what
> addresses are used, and the new device would just be added once to one
> domaindef object (just with live and config flags set) so there would be
> no need to copy it.
>
> Or, well..., after about 30 seconds of thought I realize this would lead
> to different behavior if someone e.g. deleted a currently-active device
> only from the config, then wanted to add that same device back again -
> the re-added device would end up with a different PCI address. So maybe
> that isn't the right solution either.
>
> At any rate, there is no perfect solution in sight for the current
> release, so the question is whether the new (bad) behavior is better or
> worse than the old (also bad) behavior. My understanding is that the old
> behavior could lead to a config that had two devices at the same PCI
> address, which is definitely undesirable. The new behavior could lead to
> the PCI address of a newly-added device being different the next time
> the guest is shutdown and restarted. I would tend to prefer the latter,
> with the caveat that this new behavior provides a config that works
> (from libvirt's domain parsing POV), but might create a strange error in
> the guest that would be extremely difficult to troubleshoot (especially
> 6 months from now after we've all forgotten about this patch (and
> forgotten about the idea that a more complete fix was needed).
>
> So I'm undecided about my opinion. And when undecided I tend toward
> inaction. Now *that's* helpful, isn't it?
>
Laine is stumped ;-).
I'm still stumped over what strange error could be created. How is the
virtual {PCI|SCSI} address changing any different from "real hardware"
if someone adds something new into their physical system? Or the other
fun adventure, a physical move from location A to location B where
someone "forgot" to properly label what went where and upon reassembly
things are ordered differently.
Consider some (perhaps not so) long ago SCSI devices where you'd need to
grab a small, sharp, pin like object to toggle switches to define the
address for the device when it got plugged in. That means the user had
to guess and figure out what address to use. Good luck if you conflicted
with something else. After some time it was also difficult to tell which
end was the low address toggle/bits.
Still,
Re: [libvirt] Availability of libvirt-4.6.0 Release Candidate 2
(Let's see now which people were the ones who believe it's a huge insult to Cc them in list replies, and which are the ones who appreciate the Cc and use it as a flag to raise the visibility of the message in their mail client . ?. Ah, I give up, just leaving in the Cc's and let the complaints fly)(For the record, I like explicit Cc's when you want to make sure I see something - that sends a copy to my phone, which can be mildly annoying if it's a series of 40 patches, but also very effective in getting my attention :-P) On 08/01/2018 06:40 AM, Michal Privoznik wrote: > On 07/31/2018 07:19 PM, Ján Tomko wrote: >> And I'm still unsure about leaving in >> commit 55ce65646348884656fd7bf3f109ebf8f7603494 >> qemu: Use the correct vm def on cold attach >> https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=55ce6564634 >> Which means attach-device --live --config will attach an interface >> with a different MAC address in live and persistent definition. Laine? (Let's see, you said my name once in the Cc, and once here. You really need to say it *three* times to get my attention ala Beetlejuice or Biggy Smalls. (and it helps to recite it into a mirror in a dark room, apparently)). >> > Well, It's not only MAC address that can change. The device address > might change too. This points to a broader problem. When we are parsing > a device XML we fill in the blanks in postParse callbacks. However, > those look only at either live or at inactive XML. Not at both at the > same time. So how can we fill in the blanks that would be valid for both > XMLs? Yeah, this has been a problem for a long time (it's also problematic for, e.g. the index of PCI controllers, which can cascade into differences in the PCI addresses of devices that are connected to those controllers; similar problem for other types of controllers of course). I think I even tried fixing it once, but the solution was inadequate. The problem comes when someone uses a mixture of --live only, --config only, and --live+--config device attaches/detaches. At the base of the problem is the fact that the live and config domaindef objects are stored completely independent of each other, and the code that assigns PCI addresses "seeds" the list of in-use address from a single domaindef object. Perhaps we need to merge these into a single object where each device is marked as "live", "config" or "both" - then any new device added would be guaranteed to not conflict with any existing device in live or config - the unified device list would be used to determine what addresses are used, and the new device would just be added once to one domaindef object (just with live and config flags set) so there would be no need to copy it. Or, well..., after about 30 seconds of thought I realize this would lead to different behavior if someone e.g. deleted a currently-active device only from the config, then wanted to add that same device back again - the re-added device would end up with a different PCI address. So maybe that isn't the right solution either. At any rate, there is no perfect solution in sight for the current release, so the question is whether the new (bad) behavior is better or worse than the old (also bad) behavior. My understanding is that the old behavior could lead to a config that had two devices at the same PCI address, which is definitely undesirable. The new behavior could lead to the PCI address of a newly-added device being different the next time the guest is shutdown and restarted. I would tend to prefer the latter, with the caveat that this new behavior provides a config that works (from libvirt's domain parsing POV), but might create a strange error in the guest that would be extremely difficult to troubleshoot (especially 6 months from now after we've all forgotten about this patch (and forgotten about the idea that a more complete fix was needed). So I'm undecided about my opinion. And when undecided I tend toward inaction. Now *that's* helpful, isn't it? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Matching the type of mediated devices in the migration
On Wed, 1 Aug 2018 10:22:39 + "Wang, Zhi A" wrote: > Hi: > > Let me summarize the understanding so far I got from the discussions since I > am new to this discussion. > > The mdev_type would be a generic stuff since we don't want userspace > application to be confused. The example of mdev_type is: I don't think 'generic' is the right term here. An mdev_type is a specific thing with a defined interface, we just don't define what that interface is. > There are several pre-defined mdev_types with different configurations, let's > say MDEV_TYPE A/B/C. The HW 1.0 might only support MDEV_TYPE A, the HW 2.0 > might support both MDEV_TYPE A and B, but due to HW difference, we cannot > migrate MDEV_TYPE A with HW 1.0 to MDEV_TYPE A with HW 2.0 even they have the > same MDEV_TYPE. So we need a device version either in the existing MDEV_TYPE > or a new sysfs entry. This is correct, if a foo_type_a is exposed by the same vendor driver on different hardware, then the vendor driver is guaranteeing those mdev devices are software compatible to the user. Whether the vendor driver is willing or able to support migration across the underlying hardware is a separate question. Migration compatibility and user compatibility are separate features. > Libvirt would have to check MDEV_TYPE match between source machine and > destination machine, then the device version. If any of them is different, > then it fails the migration. Device version of what? The hardware? The mdev? If the device version represents a different software interface, then the mdev type should be different. If the device version represents a migration interface compatibility then we should define it as such. > If my above understanding is correct, for VFIO part, we could define the > device version as string or a magic number. For example, the vendor mdev > driver could pass the vendor/device id and a version to VFIO and VFIO could > expose them in the UUID sysfs no matter through a new sysfs entry or through > existing MDEV_TYPE. As above, why are we trying to infer migration compatibility from a device version? What does a device version imply? What if a vendor driver wants to support cross version migration? > I prefer to expose it in the mdev_supported_types, since the libvirt node > device list could extract the device version when it enumerating the host PCI > devices or other devices, which supports mdev. We can also put it into UUID > sysfs, but the user might have to first logon the target machine and then > check the UUID and the device version by themselves, based on current code of > libvirty. I suppose all the host device management would be in node device in > libvirt, which provides remotely management of the host devices. > > For the format of a device version, an example would be: > > Vendor ID(16bit)Device ID(16bit)Class ID(16bit)Version(16bit) This is no different from the mdev type, these are user visible attributes of the device which should not change without also changing the type. Why do these necessarily convey that the migration stream is also compatible? > For string version of the device version, I guess we have to define the max > string length, which is hard to say yet. Also, a magic number is easier to be > put into the state data header during the migration. I don't think we've accomplished anything with this "device version". If anything, I think we're looking for a sysfs representation of a migration stream version where userspace would match the vendor, type, and migration stream version to determine compatibility. For vendor drivers that want to provide backwards compatibility, perhaps an optional minimum migration stream version would be provided, which would therefore imply that the format of the version can be parsed into a monotonically increasing value so that userspace can compare a stream produced by a source to a range supported by a target. Thanks, Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] cpu: split PPC64 map data into separate files
Signed-off-by: Daniel P. Berrangé --- src/cpu/cpu_map.xml | 41 + src/cpu/cpu_map_ppc64_POWER6.xml| 6 src/cpu/cpu_map_ppc64_POWER7.xml| 7 + src/cpu/cpu_map_ppc64_POWER8.xml| 8 + src/cpu/cpu_map_ppc64_POWER9.xml| 6 src/cpu/cpu_map_ppc64_POWERPC_e5500.xml | 6 src/cpu/cpu_map_ppc64_POWERPC_e6500.xml | 6 src/cpu/cpu_map_ppc64_vendors.xml | 4 +++ 8 files changed, 50 insertions(+), 34 deletions(-) create mode 100644 src/cpu/cpu_map_ppc64_POWER6.xml create mode 100644 src/cpu/cpu_map_ppc64_POWER7.xml create mode 100644 src/cpu/cpu_map_ppc64_POWER8.xml create mode 100644 src/cpu/cpu_map_ppc64_POWER9.xml create mode 100644 src/cpu/cpu_map_ppc64_POWERPC_e5500.xml create mode 100644 src/cpu/cpu_map_ppc64_POWERPC_e6500.xml create mode 100644 src/cpu/cpu_map_ppc64_vendors.xml diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 9af190a579..e236c41733 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -2340,43 +2340,16 @@ - - - + - - - - - - - - - - - - - - - - - - - - - - + + + + - - - - - - - - - + + diff --git a/src/cpu/cpu_map_ppc64_POWER6.xml b/src/cpu/cpu_map_ppc64_POWER6.xml new file mode 100644 index 00..00e27495f4 --- /dev/null +++ b/src/cpu/cpu_map_ppc64_POWER6.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/cpu/cpu_map_ppc64_POWER7.xml b/src/cpu/cpu_map_ppc64_POWER7.xml new file mode 100644 index 00..a071481805 --- /dev/null +++ b/src/cpu/cpu_map_ppc64_POWER7.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/src/cpu/cpu_map_ppc64_POWER8.xml b/src/cpu/cpu_map_ppc64_POWER8.xml new file mode 100644 index 00..64d96fc4c4 --- /dev/null +++ b/src/cpu/cpu_map_ppc64_POWER8.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/src/cpu/cpu_map_ppc64_POWER9.xml b/src/cpu/cpu_map_ppc64_POWER9.xml new file mode 100644 index 00..149fcde924 --- /dev/null +++ b/src/cpu/cpu_map_ppc64_POWER9.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/cpu/cpu_map_ppc64_POWERPC_e5500.xml b/src/cpu/cpu_map_ppc64_POWERPC_e5500.xml new file mode 100644 index 00..3d64c8926c --- /dev/null +++ b/src/cpu/cpu_map_ppc64_POWERPC_e5500.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/cpu/cpu_map_ppc64_POWERPC_e6500.xml b/src/cpu/cpu_map_ppc64_POWERPC_e6500.xml new file mode 100644 index 00..b0d1006076 --- /dev/null +++ b/src/cpu/cpu_map_ppc64_POWERPC_e6500.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/src/cpu/cpu_map_ppc64_vendors.xml b/src/cpu/cpu_map_ppc64_vendors.xml new file mode 100644 index 00..52ad45c0bd --- /dev/null +++ b/src/cpu/cpu_map_ppc64_vendors.xml @@ -0,0 +1,4 @@ + + + + -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] cpu: modularize the CPU map data file
Currently we have a cpu_map.xml file that contains all the features and CPU models for all architectures in one place. I frequently find myself wondering about the differences between CPU models, but it is hard to compare them as the list of features is huge. With this patch series we end up with a large set of small files, one per named CPU model, along with one for the feature and vendor definitions cpu_map_ppc64_POWER6.xml cpu_map_ppc64_POWER7.xml cpu_map_ppc64_POWER8.xml cpu_map_ppc64_POWER9.xml cpu_map_ppc64_POWERPC_e5500.xml cpu_map_ppc64_POWERPC_e6500.xml cpu_map_ppc64_vendors.xml cpu_map_x86_486.xml cpu_map_x86_athlon.xml cpu_map_x86_Broadwell-IBRS.xml cpu_map_x86_Broadwell-noTSX-IBRS.xml cpu_map_x86_Broadwell-noTSX.xml cpu_map_x86_Broadwell.xml cpu_map_x86_Conroe.xml cpu_map_x86_core2duo.xml cpu_map_x86_coreduo.xml cpu_map_x86_cpu64-rhel5.xml cpu_map_x86_cpu64-rhel6.xml cpu_map_x86_EPYC-IBRS.xml cpu_map_x86_EPYC.xml cpu_map_x86_features.xml cpu_map_x86_Haswell-IBRS.xml cpu_map_x86_Haswell-noTSX-IBRS.xml cpu_map_x86_Haswell-noTSX.xml cpu_map_x86_Haswell.xml cpu_map_x86_IvyBridge-IBRS.xml cpu_map_x86_IvyBridge.xml cpu_map_x86_kvm32.xml cpu_map_x86_kvm64.xml cpu_map_x86_n270.xml cpu_map_x86_Nehalem-IBRS.xml cpu_map_x86_Nehalem.xml cpu_map_x86_Opteron_G1.xml cpu_map_x86_Opteron_G2.xml cpu_map_x86_Opteron_G3.xml cpu_map_x86_Opteron_G4.xml cpu_map_x86_Opteron_G5.xml cpu_map_x86_Penryn.xml cpu_map_x86_pentium2.xml cpu_map_x86_pentium3.xml cpu_map_x86_pentiumpro.xml cpu_map_x86_pentium.xml cpu_map_x86_phenom.xml cpu_map_x86_qemu32.xml cpu_map_x86_qemu64.xml cpu_map_x86_SandyBridge-IBRS.xml cpu_map_x86_SandyBridge.xml cpu_map_x86_Skylake-Client-IBRS.xml cpu_map_x86_Skylake-Client.xml cpu_map_x86_Skylake-Server-IBRS.xml cpu_map_x86_Skylake-Server.xml cpu_map_x86_vendors.xml cpu_map_x86_Westmere-IBRS.xml cpu_map_x86_Westmere.xml The main cpu_map.xml file is now just a list of statements to pull in the individual files Now we can easily see the differences in each model: $ diff cpu_map_x86_Broadwell.xml cpu_map_x86_Skylake-Client.xml 2,3c2,3 < < --- > > 5a6 > 8a10 > 18a21 > 30a34 > 42a47 > 56a62 > 57a64 > 58a66,67 > > Daniel P. Berrangé (4): cpu: allow include files for CPU definition cpu: push more parsing logic into common code cpu: split PPC64 map data into separate files cpu: split x86 map data into separate files libvirt.spec.in |2 +- mingw-libvirt.spec.in|4 +- src/Makefile.am |2 +- src/cpu/cpu_map.c| 160 +- src/cpu/cpu_map.h| 22 +- src/cpu/cpu_map.xml | 2415 +- src/cpu/cpu_map_ppc64_POWER6.xml |6 + src/cpu/cpu_map_ppc64_POWER7.xml |7 + src/cpu/cpu_map_ppc64_POWER8.xml |8 + src/cpu/cpu_map_ppc64_POWER9.xml |6 + src/cpu/cpu_map_ppc64_POWERPC_e5500.xml |6 + src/cpu/cpu_map_ppc64_POWERPC_e6500.xml |6 + src/cpu/cpu_map_ppc64_vendors.xml|4 + src/cpu/cpu_map_x86_486.xml |7 + src/cpu/cpu_map_x86_Broadwell-IBRS.xml | 61 + src/cpu/cpu_map_x86_Broadwell-noTSX-IBRS.xml | 59 + src/cpu/cpu_map_x86_Broadwell-noTSX.xml | 58 + src/cpu/cpu_map_x86_Broadwell.xml| 60 + src/cpu/cpu_map_x86_Conroe.xml | 33 + src/cpu/cpu_map_x86_EPYC-IBRS.xml| 73 + src/cpu/cpu_map_x86_EPYC.xml | 72 + src/cpu/cpu_map_x86_Haswell-IBRS.xml | 57 + src/cpu/cpu_map_x86_Haswell-noTSX-IBRS.xml | 55 + src/cpu/cpu_map_x86_Haswell-noTSX.xml| 54 + src/cpu/cpu_map_x86_Haswell.xml | 56 + src/cpu/cpu_map_x86_IvyBridge-IBRS.xml | 51 + src/cpu/cpu_map_x86_IvyBridge.xml| 50 + src/cpu/cpu_map_x86_Nehalem-IBRS.xml | 38 + src/cpu/cpu_map_x86_Nehalem.xml | 37 + src/cpu/cpu_map_x86_Opteron_G1.xml | 31 + src/cpu/cpu_map_x86_Opteron_G2.xml | 35 + src/cpu/cpu_map_x86_Opteron_G3.xml | 40 + src/cpu/cpu_map_x86_Opteron_G4.xml | 50 + src/cpu/cpu_map_x86_Opteron_G5.xml | 53 + src/cpu/cpu_map_x86_Penryn.xml | 35 + src/cpu/cpu_map_x86_SandyBridge-IBRS.xml | 45 + src/cpu/cpu_map_x86_SandyBridge.xml | 44 + src/cpu/cpu_map_x86_Skylake-Client-IBRS.xml | 70 + src/cpu/cpu_map_x86_Skylake-Client.xml | 69 + src/cpu/cpu_map_x86_Skylake-Server-IBRS.xml | 77 + src/cpu/cpu_map_x86_Skylake-Server.xml | 76 +
[libvirt] [PATCH 1/4] cpu: allow include files for CPU definition
Allow for syntax
to reference other files in the CPU database directory
Signed-off-by: Daniel P. Berrangé
---
libvirt.spec.in | 2 +-
mingw-libvirt.spec.in | 4 +--
src/Makefile.am | 2 +-
src/cpu/cpu_map.c | 84 +--
4 files changed, 86 insertions(+), 6 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 19ae55cdaf..b6745dbffa 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1856,7 +1856,7 @@ exit 0
%{_datadir}/libvirt/schemas/storagepool.rng
%{_datadir}/libvirt/schemas/storagevol.rng
-%{_datadir}/libvirt/cpu_map.xml
+%{_datadir}/libvirt/cpu_map*.xml
%{_datadir}/libvirt/test-screenshot.png
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index cc1e619927..22fe7a000f 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -260,7 +260,7 @@ rm -rf
$RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
%{mingw32_datadir}/libvirt/api/libvirt-qemu-api.xml
%{mingw32_datadir}/libvirt/api/libvirt-admin-api.xml
-%{mingw32_datadir}/libvirt/cpu_map.xml
+%{mingw32_datadir}/libvirt/cpu_map*.xml
%{mingw32_datadir}/libvirt/test-screenshot.png
@@ -347,7 +347,7 @@ rm -rf
$RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
%{mingw64_datadir}/libvirt/api/libvirt-qemu-api.xml
%{mingw64_datadir}/libvirt/api/libvirt-admin-api.xml
-%{mingw64_datadir}/libvirt/cpu_map.xml
+%{mingw64_datadir}/libvirt/cpu_map*.xml
%{mingw64_datadir}/libvirt/test-screenshot.png
diff --git a/src/Makefile.am b/src/Makefile.am
index a4f213480e..11a7ac81e2 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -366,7 +366,7 @@ check-local: check-protocol check-symfile check-symsorting \
-pkgdata_DATA = cpu/cpu_map.xml
+pkgdata_DATA = $(wildcard $(srcdir)/cpu/cpu_map*.xml)
EXTRA_DIST += $(pkgdata_DATA)
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index d263eb8cdd..9e090919ed 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -70,6 +70,83 @@ static int load(xmlXPathContextPtr ctxt,
return ret;
}
+static int
+cpuMapLoadInclude(const char *filename,
+ cpuMapLoadCallback cb,
+ void *data)
+{
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
+int ret = -1;
+int element;
+char *mapfile;
+
+if (!(mapfile = virFileFindResource(filename,
+abs_topsrcdir "/src/cpu",
+PKGDATADIR)))
+return -1;
+
+VIR_DEBUG("Loading CPU map include from %s", mapfile);
+
+if (!(xml = virXMLParseFileCtxt(mapfile, )))
+goto cleanup;
+
+ctxt->node = xmlDocGetRootElement(xml);
+
+for (element = 0; element < CPU_MAP_ELEMENT_LAST; element++) {
+if (load(ctxt, element, cb, data) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse CPU map '%s'"), mapfile);
+goto cleanup;
+}
+}
+
+ret = 0;
+
+ cleanup:
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+VIR_FREE(mapfile);
+
+return ret;
+}
+
+
+static int loadIncludes(xmlXPathContextPtr ctxt,
+cpuMapLoadCallback callback,
+void *data)
+{
+int ret = -1;
+xmlNodePtr ctxt_node;
+xmlNodePtr *nodes = NULL;
+int n;
+size_t i;
+
+ctxt_node = ctxt->node;
+
+n = virXPathNodeSet("include", ctxt, );
+if (n < 0)
+goto cleanup;
+
+for (i = 0; i < n; i++) {
+char *filename = virXMLPropString(nodes[i], "filename");
+VIR_DEBUG("Finding CPU map include '%s'", filename);
+if (cpuMapLoadInclude(filename, callback, data) < 0) {
+VIR_FREE(filename);
+goto cleanup;
+}
+VIR_FREE(filename);
+}
+
+ret = 0;
+
+ cleanup:
+ctxt->node = ctxt_node;
+VIR_FREE(nodes);
+
+return ret;
+}
+
int cpuMapLoad(const char *arch,
cpuMapLoadCallback cb,
@@ -88,7 +165,7 @@ int cpuMapLoad(const char *arch,
PKGDATADIR)))
return -1;
-VIR_DEBUG("Loading CPU map from %s", mapfile);
+VIR_DEBUG("Loading '%s' CPU map from %s", arch, mapfile);
if (arch == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -122,11 +199,14 @@ int cpuMapLoad(const char *arch,
for (element = 0; element < CPU_MAP_ELEMENT_LAST; element++) {
if (load(ctxt, element, cb, data) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse CPU map for %s architecture"),
arch);
+ _("cannot parse CPU map '%s'"), mapfile);
goto cleanup;
}
}
+if (loadIncludes(ctxt, cb, data) < 0)
+goto cleanup;
+
ret = 0;
cleanup:
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] ESX: Fixing SetAutoStart
This is a new version from the last patchset sent yesterday, but now using VIR_STRNDUP, instead of allocating memory manually. First version: https://www.redhat.com/archives/libvir-list/2018-August/msg0.html Marcos Paulo de Souza (2): esx: Do not crash SetAutoStart by double free esx: Fix SetAutoStart invalid pointer free src/esx/esx_driver.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] esx: Fix SetAutoStart invalid pointer free
esxVI_AutoStartPowerInfo_Free, which is called from
esxVI_HostAutoStartManagerConfig_Free, will always call VIR_FREE to free
memory from {start,stop}Action, leading to a invalid pointer.
With this patch applied, ESX can set autostart successfully to all it's
domains.
Signed-off-by: Marcos Paulo de Souza
---
Changes from v1:
* Stop calling VIR_ALLOC_N and strcpy, and use VIR_STRNDUP instead
src/esx/esx_driver.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 3835e4cb3c..dc07cf8770 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3394,9 +3394,15 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
newPowerInfo->startOrder->value = -1; /* no specific start order */
newPowerInfo->startDelay->value = -1; /* use system default */
newPowerInfo->waitForHeartbeat =
esxVI_AutoStartWaitHeartbeatSetting_SystemDefault;
-newPowerInfo->startAction = autostart ? (char *)"powerOn" : (char *)"none";
newPowerInfo->stopDelay->value = -1; /* use system default */
-newPowerInfo->stopAction = (char *)"none";
+
+/* startAction and stopAction will be freed by
esxVI_HostAutoStartManagerConfig_Free */
+if (VIR_STRNDUP(newPowerInfo->startAction, autostart ? "powerOn" : "none",
+autostart ? 7 : 4) < 1)
+goto cleanup;
+
+if (VIR_STRNDUP(newPowerInfo->stopAction, "none", 4) < 1)
+goto cleanup;
if (esxVI_AutoStartPowerInfo_AppendToList(>powerInfo,
newPowerInfo) < 0) {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of libvirt-4.6.0 Release Candidate 2
On 08/01/2018 12:47 PM, Peter Krempa wrote: > On Wed, Aug 01, 2018 at 12:40:14 +0200, Michal Privoznik wrote: >> On 07/31/2018 07:19 PM, Ján Tomko wrote: >>> And I'm still unsure about leaving in >>> commit 55ce65646348884656fd7bf3f109ebf8f7603494 >>> qemu: Use the correct vm def on cold attach >>> https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=55ce6564634 >>> Which means attach-device --live --config will attach an interface >>> with a different MAC address in live and persistent definition. Laine? >>> >> >> Well, It's not only MAC address that can change. The device address >> might change too. This points to a broader problem. When we are parsing >> a device XML we fill in the blanks in postParse callbacks. However, >> those look only at either live or at inactive XML. Not at both at the >> same time. So how can we fill in the blanks that would be valid for both >> XMLs? > > We can fill in the definition and then copy it and validate it > afterwards. That way the blanks are filled and we can then validate that > it fits into the other definition. The description here sounds that in > this release we made things worse than it was before though. But that might fail. For instance, we generate a PCI for a hot plugged device based on say live XML, but the address is already taken in config XML. I'm not sure if there's an easy way out of this. And regarding the revert - I guess it's just a matter of somebody posting the patch. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for v4.6.0 0/2] Fix some random problems
On 08/01/2018 07:57 AM, Bjoern Walk wrote: And here's the fix for the viriscsitest on big-endian machine like Daniel suggested. From d59b254294a90c5a9ca0fb6ad29465cd0950bb61 Mon Sep 17 00:00:00 2001 From: Bjoern Walk Date: Wed, 1 Aug 2018 14:48:47 +0200 Subject: [PATCH] util: virrandom: make virRandomBits endian-safe Make the generation of random bits in virRandomBits independent of the endianness of the running architecture. This also solves problems with the mocked random byte generation on big-endian machines. Suggested-by: Daniel P. Berrangé Signed-off-by: Bjoern Walk --- src/util/virrandom.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) -ret &= (1ULL << nbits) - 1; -return ret; +return virReadBufInt64LE(ret) & ((1ULL << nbits) - 1); Needs to be rebased on top of the fix that (1ULL << 64) is not defined. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 5/8] guests: Drop install_config from inventory
On Wed, 2018-08-01 at 14:46 +0200, Erik Skultety wrote:
> On Thu, Jul 19, 2018 at 06:32:05PM +0200, Andrea Bolognani wrote:
> > # preseed files must use a well-known name to be picked up by
> > # d-i; for kickstart files, we can use whatever name we please
> > # but we need to point anaconda in the right direction through
> > # a kernel argument
> > -extra_arg = "console=ttyS0 ks=file:/{}".format(
> > -facts["install_config"],
> > -)
> > +extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
>
> Pre-existing, but ks= should be replaced by inst.ks, as using the inst. prefix
> is the current preferred way of passing installer-specific options, the legacy
> syntax is still accepted, however, this may be a subject to future changes -
> the inst. prefix is supported since fedora 17 and RHEL/CentOS 7 and since we
> dropped CentOS 6 from the supported distros, we don't need to keep the legacy
> option :).
Today I learned! :)
We can definitely switch to the non-legacy version, and I suspect
that if we spent some time on it we might be able to modernize the
kickstart file itself a bit as well...
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
Pino Toscano [2018-08-01, 03:08PM +0200]: > viriscsitest tries to ensure the interface IQN used is a specific one, > checking later on that it is the same all during the whole test. Since > the IQN generation involves random bytes, viriscsitest got a fake > virRandomBytes from the virrandommock helper library, setting static > values. virRandomBits(), called by virStorageBackendCreateIfaceIQN(), > always requests 8 random bytes, chopping off the ones not requested by > the caller -- this meant that on big endian machines it would chop bytes > from the wrong size of the data buffer, and thus not returning the > expected numbers. > > As a fix, do not rely on the mock virRandomBytes, but provide an own > version of it: this version will fill the values in the expected order, > depending on the endianness of the system. This way, the result of > virStorageBackendCreateIfaceIQN() will be what the test actually > expects. > > Signed-off-by: Pino Toscano Tested-by: Bjoern Walk signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
On Wednesday, 1 August 2018 15:24:05 CEST Daniel P. Berrangé wrote: > On Wed, Aug 01, 2018 at 03:08:07PM +0200, Pino Toscano wrote: > > viriscsitest tries to ensure the interface IQN used is a specific one, > > checking later on that it is the same all during the whole test. Since > > the IQN generation involves random bytes, viriscsitest got a fake > > virRandomBytes from the virrandommock helper library, setting static > > values. virRandomBits(), called by virStorageBackendCreateIfaceIQN(), > > always requests 8 random bytes, chopping off the ones not requested by > > the caller -- this meant that on big endian machines it would chop bytes > > from the wrong size of the data buffer, and thus not returning the > > expected numbers. > > > > As a fix, do not rely on the mock virRandomBytes, but provide an own > > version of it: this version will fill the values in the expected order, > > depending on the endianness of the system. This way, the result of > > virStorageBackendCreateIfaceIQN() will be what the test actually > > expects. > > > > Signed-off-by: Pino Toscano > > Mailman is having deliver problems so you might not have received > the patch that was sent earlier for this problem: > > https://www.redhat.com/archives/libvir-list/2018-August/msg00039.html > > I prefer that approach since it avoids the endianness problems for > all future usage too. I replied to that patch, as IMHO there is no "endianness" for random data, and thus virRandomBytes() ought to not do any manipulation. -- 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
Re: [libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
On Wed, Aug 01, 2018 at 03:08:07PM +0200, Pino Toscano wrote: > viriscsitest tries to ensure the interface IQN used is a specific one, > checking later on that it is the same all during the whole test. Since > the IQN generation involves random bytes, viriscsitest got a fake > virRandomBytes from the virrandommock helper library, setting static > values. virRandomBits(), called by virStorageBackendCreateIfaceIQN(), > always requests 8 random bytes, chopping off the ones not requested by > the caller -- this meant that on big endian machines it would chop bytes > from the wrong size of the data buffer, and thus not returning the > expected numbers. > > As a fix, do not rely on the mock virRandomBytes, but provide an own > version of it: this version will fill the values in the expected order, > depending on the endianness of the system. This way, the result of > virStorageBackendCreateIfaceIQN() will be what the test actually > expects. > > Signed-off-by: Pino Toscano Mailman is having deliver problems so you might not have received the patch that was sent earlier for this problem: https://www.redhat.com/archives/libvir-list/2018-August/msg00039.html I prefer that approach since it avoids the endianness problems for all future usage too. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 8/8] guests: Update documentation
On Thu, Jul 19, 2018 at 06:32:08PM +0200, Andrea Bolognani wrote: > The usage has once again changed slightly; additionally, > a few concrete examples are now provided. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 7/8] lcitool: Allow installing a subset of build dependencies
On Thu, Jul 19, 2018 at 06:32:07PM +0200, Andrea Bolognani wrote: > For CentOS CI, we need build dependencies for all known > projects to be installed; however, when using lcitool > for development purposes, it is very convenient to install > just the subset relevant to the project that's being > worked on, as doing so reduces the storage requirements > and makes the update procedure quite a bit faster. > > The previous behavior can still be obtained by using > > $ lcitool -a update -p all ... > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 6/8] guests: Move install configs
On Thu, Jul 19, 2018 at 06:32:06PM +0200, Andrea Bolognani wrote: > The rationale is the same as for moving playbooks. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 3/8] lcitool: Pass base and playbook_base to Ansible
On Thu, Jul 19, 2018 at 06:32:03PM +0200, Andrea Bolognani wrote: > We want to get rid of relative paths in playbooks and > tasks, and in order to do that we have to provide Ansible > with some more information. > > base is the directory where lcitool lives, and > playbook_base is the directory where a playbook should > look for its private resources: they match for the time > being, but that will no longer be the case very shortly. > > Signed-off-by: Andrea Bolognani > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: viriscsitest: make it work on big endian archs
viriscsitest tries to ensure the interface IQN used is a specific one,
checking later on that it is the same all during the whole test. Since
the IQN generation involves random bytes, viriscsitest got a fake
virRandomBytes from the virrandommock helper library, setting static
values. virRandomBits(), called by virStorageBackendCreateIfaceIQN(),
always requests 8 random bytes, chopping off the ones not requested by
the caller -- this meant that on big endian machines it would chop bytes
from the wrong size of the data buffer, and thus not returning the
expected numbers.
As a fix, do not rely on the mock virRandomBytes, but provide an own
version of it: this version will fill the values in the expected order,
depending on the endianness of the system. This way, the result of
virStorageBackendCreateIfaceIQN() will be what the test actually
expects.
Signed-off-by: Pino Toscano
---
tests/viriscsitest.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c
index 4fd7ff6e19..d43c0115d3 100644
--- a/tests/viriscsitest.c
+++ b/tests/viriscsitest.c
@@ -21,6 +21,7 @@
#include
#include "testutils.h"
+#include "virrandom.h"
#ifdef WIN32
int
@@ -36,6 +37,32 @@ main(void)
# define VIR_FROM_THIS VIR_FROM_NONE
+bool isLittleEndian(void);
+bool isLittleEndian(void)
+{
+static int cache = -1;
+if (cache < 0) {
+int val = 1;
+char *ptr = (char *)
+cache = *ptr ? 1 : 0;
+}
+return cache > 0;
+}
+
+
+int
+virRandomBytes(unsigned char *buf,
+ size_t buflen)
+{
+size_t i;
+const bool isLE = isLittleEndian();
+
+for (i = 0; i < buflen; i++)
+buf[i] = isLE ? i : buflen - i - 1;
+
+return 0;
+}
+
static const char *iscsiadmSessionOutput =
"tcp: [1] 10.20.30.40:3260,1 iqn.2004-06.example:example1:iscsi.test\n"
"tcp: [2] 10.20.30.41:3260,1 iqn.2005-05.example:example1:iscsi.hello\n"
@@ -368,6 +395,5 @@ mymain(void)
return EXIT_SUCCESS;
}
-VIR_TEST_MAIN_PRELOAD(mymain,
- abs_builddir "/.libs/virrandommock.so")
+VIR_TEST_MAIN(mymain)
#endif /* WIN32 */
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for v4.6.0 0/2] Fix some random problems
And here's the fix for the viriscsitest on big-endian machine like
Daniel suggested.
Bjoern
--
IBM Systems
Linux on Z & Virtualization Development
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
From d59b254294a90c5a9ca0fb6ad29465cd0950bb61 Mon Sep 17 00:00:00 2001
From: Bjoern Walk
Date: Wed, 1 Aug 2018 14:48:47 +0200
Subject: [PATCH] util: virrandom: make virRandomBits endian-safe
Make the generation of random bits in virRandomBits independent of the
endianness of the running architecture.
This also solves problems with the mocked random byte generation on
big-endian machines.
Suggested-by: Daniel P. Berrangé
Signed-off-by: Bjoern Walk
---
src/util/virrandom.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virrandom.c b/src/util/virrandom.c
index 01cc82a0..a58ee97a 100644
--- a/src/util/virrandom.c
+++ b/src/util/virrandom.c
@@ -34,6 +34,7 @@
# include
#endif
+#include "virendian.h"
#include "virrandom.h"
#include "virthread.h"
#include "count-one-bits.h"
@@ -60,16 +61,15 @@ VIR_LOG_INIT("util.random");
*/
uint64_t virRandomBits(int nbits)
{
-uint64_t ret = 0;
+uint8_t ret[8];
-if (virRandomBytes((unsigned char *) , sizeof(ret)) < 0) {
+if (virRandomBytes(ret, sizeof(ret)) < 0) {
/* You're already hosed, so this particular non-random value
* isn't any worse. */
return 0;
}
-ret &= (1ULL << nbits) - 1;
-return ret;
+return virReadBufInt64LE(ret) & ((1ULL << nbits) - 1);
}
--
2.17.0
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] conf: virDomainDefValidateInternal prohibit some characters in shmem name
On 08/01/2018 06:24 AM, skob...@redhat.com wrote:
> On Fri, 2018-07-27 at 13:19 -0400, John Ferlan wrote:
>> You shouldn't drop libvir-list when you reply...
> Sorry, I dropped it by mistake.
>>
>> On 07/27/2018 06:35 AM, skob...@redhat.com wrote:
>>> On Mon, 2018-07-23 at 17:32 -0400, John Ferlan wrote:
On 07/12/2018 09:10 AM, Simon Kobyda wrote:
> XML shmem name will not include character '/', and will not be
> equal to strings
> "." or "..", as shmem name is used in a path.
Validate that the provided XML shmem name is not directory
specific
"."
or ".." names as well as ensuring that there is no path separator
'/'
in
the name.
> https://bugzilla.redhat.com/show_bug.cgi?id=1192400
> ---
>
> Changes in V2
> - Added error reports
> - Error situation will happen only if shmem name is
> equal to
> "." or "..", however their occurence in a name
> compromised of
> more
> characters is allowed.
>
> src/conf/domain_conf.c | 22 ++
> 1 file changed, 22 insertions(+)
>
I believe this actually belongs in
virDomainDeviceDefValidateInternal
for case VIR_DOMAIN_DEVICE_SHMEM.
Also, should the docs/schemas/domaincommon.rng be modified?
Currently
it
has:
[^/]*
>>>
>>> Is it a good idea? To create such regular expression, I believe it
>>> would make it unreadable for another person, ergo useless. I don't
>>> want
>>> it to end up as IPv6. I've benn told that actual parser can be, and
>>> sometimes is stricter than scheme.
>>>
>>
>> Well this is what I was hoping others would be able to chime in on.
>> Hence why you ask on list. I don't disagree that being stricter in
>> Parse
>> is fine. Doing regexes is like reading a foreign language to me at
>> times. Some just don't make sense.
>
> Yeah, that's why I don't want to try to insert complicated regex in
> docs/schemas/domaincommon.rng.
>>
>> I think that regex is indicating - any character except '/', but I'm
>> not
>> sure. If it is, it's ironic that we have to check for '/'
>> specifically
>> since it shouldn't be passing the xml validation test - OK at least
>> if
>> one was editing via virsh.
>>
>> John
>>
> You are right about what that regex does. But from what I heard, that
> functionality is rather new to libvirt, so users have option to bypass
> that verification. So I would like to also leave it in code in case
> users try to bypass that regex verification.
>
> Simon.
>
See commit id '1c06d0fa' for the shmem validation previously added.
Ironically a higher numbered bz than referenced your commit.
And yes it's quite easy to tell virsh edit to ignore the xml validation
error. I'm fine with leaving the RNG as is. I had a different bz in my
pile related to resource names, so the topic was somewhat fresh in mind
related to looking at what various RNG "patterns" had, see:
https://www.redhat.com/archives/libvir-list/2018-July/msg02046.html
in that case the problem is that it's possible to name something " " or
" " and you don't know whether a space or tab or any combination was
used for the entirety of the name.
I think if you move the code from virDomainDefValidateInternal to a
SHMEM specific case in virDomainDeviceDefValidateInternal, then that'll
be fine. I would think that the qemuProcessStartValidateShmem call from
commit 1c06d0fa would not be possible then.
John
Consider how other names are limited in their scope. The
basictypes.rng
has a number of examples.
Naturally, the problem with changing it is that someone somewhere
will
complain, but libvirt used to accept this other format. Right now
I
would think the scope a bit too broad.
>>>
>>> But then we cannot fix most of the bugs, since there could always
>>> be
>>> some user which relies on bug behavior.
If we are to limit the name we should also document in
docs/formatdomain.html.in that the shmem name is "limited" in
name to
avoid the '/' character, ".", and "..".
BTW: My regex isn't that good, but it would seem '/' is an
invalid
character by XML standards even though the code never checked for
it.
Using virt-xml-validate would "validate" whether
someone
provides valid XML.
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7ab2953d83..6b34c17de4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const
> virDomainDef *def)
> static int
> virDomainDefValidateInternal(const virDomainDef *def)
> {
> +size_t i;
> +
> if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
> return -1;
>
> @@ -6136,6 +6138,26
Re: [libvirt] [jenkins-ci PATCH 5/8] guests: Drop install_config from inventory
On Thu, Jul 19, 2018 at 06:32:05PM +0200, Andrea Bolognani wrote: > The information is mostly duplicated and can be easily > inferred in a programmatic manner, so storing it in the > inventory is far from the cleanest solution. > > As a side-effect, we reintroduce the error message that > was supposed to be displayed when attempting to install > a FreeBSD guest but was lost in the Python rewrite. > > Signed-off-by: Andrea Bolognani > --- > guests/host_vars/libvirt-centos-7/install.yml | 1 - > guests/host_vars/libvirt-debian-8/install.yml | 1 - > guests/host_vars/libvirt-debian-9/install.yml | 1 - > .../host_vars/libvirt-debian-sid/install.yml | 1 - > .../host_vars/libvirt-fedora-27/install.yml | 1 - > .../host_vars/libvirt-fedora-28/install.yml | 1 - > .../libvirt-fedora-rawhide/install.yml| 1 - > .../host_vars/libvirt-ubuntu-16/install.yml | 1 - > .../host_vars/libvirt-ubuntu-18/install.yml | 1 - > guests/lcitool| 19 ++- > 10 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/guests/host_vars/libvirt-centos-7/install.yml > b/guests/host_vars/libvirt-centos-7/install.yml > index f003b89..2164ac5 100644 > --- a/guests/host_vars/libvirt-centos-7/install.yml > +++ b/guests/host_vars/libvirt-centos-7/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: http://mirror.centos.org/centos/7/os/x86_64/ > -install_config: kickstart.cfg > diff --git a/guests/host_vars/libvirt-debian-8/install.yml > b/guests/host_vars/libvirt-debian-8/install.yml > index a2c8341..299a1a6 100644 > --- a/guests/host_vars/libvirt-debian-8/install.yml > +++ b/guests/host_vars/libvirt-debian-8/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: http://deb.debian.org/debian/dists/jessie/main/installer-amd64/ > -install_config: preseed.cfg > diff --git a/guests/host_vars/libvirt-debian-9/install.yml > b/guests/host_vars/libvirt-debian-9/install.yml > index 5b1da76..7641753 100644 > --- a/guests/host_vars/libvirt-debian-9/install.yml > +++ b/guests/host_vars/libvirt-debian-9/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: http://deb.debian.org/debian/dists/stretch/main/installer-amd64/ > -install_config: preseed.cfg > diff --git a/guests/host_vars/libvirt-debian-sid/install.yml > b/guests/host_vars/libvirt-debian-sid/install.yml > index da1c7a8..46c6366 100644 > --- a/guests/host_vars/libvirt-debian-sid/install.yml > +++ b/guests/host_vars/libvirt-debian-sid/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: http://deb.debian.org/debian/dists/sid/main/installer-amd64/ > -install_config: preseed.cfg > diff --git a/guests/host_vars/libvirt-fedora-27/install.yml > b/guests/host_vars/libvirt-fedora-27/install.yml > index 66ce38e..f7a45af 100644 > --- a/guests/host_vars/libvirt-fedora-27/install.yml > +++ b/guests/host_vars/libvirt-fedora-27/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: > https://download.fedoraproject.org/pub/fedora/linux/releases/27/Everything/x86_64/os > -install_config: kickstart.cfg > diff --git a/guests/host_vars/libvirt-fedora-28/install.yml > b/guests/host_vars/libvirt-fedora-28/install.yml > index 4b2b9f0..73433f1 100644 > --- a/guests/host_vars/libvirt-fedora-28/install.yml > +++ b/guests/host_vars/libvirt-fedora-28/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: > https://download.fedoraproject.org/pub/fedora/linux/releases/28/Everything/x86_64/os > -install_config: kickstart.cfg > diff --git a/guests/host_vars/libvirt-fedora-rawhide/install.yml > b/guests/host_vars/libvirt-fedora-rawhide/install.yml > index 2216e81..5c67562 100644 > --- a/guests/host_vars/libvirt-fedora-rawhide/install.yml > +++ b/guests/host_vars/libvirt-fedora-rawhide/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: > https://download.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/x86_64/os > -install_config: kickstart.cfg > diff --git a/guests/host_vars/libvirt-ubuntu-16/install.yml > b/guests/host_vars/libvirt-ubuntu-16/install.yml > index a7bb2da..d8ce841 100644 > --- a/guests/host_vars/libvirt-ubuntu-16/install.yml > +++ b/guests/host_vars/libvirt-ubuntu-16/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: > http://archive.ubuntu.com/ubuntu/dists/xenial/main/installer-amd64/ > -install_config: preseed.cfg > diff --git a/guests/host_vars/libvirt-ubuntu-18/install.yml > b/guests/host_vars/libvirt-ubuntu-18/install.yml > index bd3e1d9..544b3f2 100644 > --- a/guests/host_vars/libvirt-ubuntu-18/install.yml > +++ b/guests/host_vars/libvirt-ubuntu-18/install.yml > @@ -1,3 +1,2 @@ > --- > install_url: > http://archive.ubuntu.com/ubuntu/dists/bionic/main/installer-amd64/ > -install_config: preseed.cfg > diff --git a/guests/lcitool b/guests/lcitool > index 2cfb0e9..13f0392 100755 > --- a/guests/lcitool > +++ b/guests/lcitool > @@ -382,15 +382,24 @@ class Application: > facts["install_network"], > ) > > -install_config = os.path.join(base,
Re: [libvirt] [PATCH 2/2] viriscsi: Request more random bits for interface name
On Wed, Aug 01, 2018 at 01:44:33PM +0200, Michal Privoznik wrote:
> In virStorageBackendCreateIfaceIQN() the virRandomBits() is
> called in order to use random bits to generate random name for
> new interface. However, virAsprintf() is expecting 32 bits and we
> are requesting only 30.
>
> Signed-off-by: Michal Privoznik
> ---
> src/util/viriscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 653b4fd932..f00aeb53a7 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -221,7 +221,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
>
> if (virAsprintf(_ifacename,
> "libvirt-iface-%08llx",
> -(unsigned long long)virRandomBits(30)) < 0)
> +(unsigned long long)virRandomBits(32)) < 0)
> return -1;
>
> VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'",
Reviewed-by: Daniel P. Berrangé
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: Don't overflow in virRandomBits
On Wed, Aug 01, 2018 at 01:44:32PM +0200, Michal Privoznik wrote: > The function is supposed to return up to 64bit long integer. In > order to do that it calls virRandomBytes() to fill the integer > with random bytes and then masks out everything but requested > bits. However, when doing that it shifts 1U and not 1ULL. So > effectively, requesting 32 random bis or more always return 0 > which is not random enough. > > Signed-off-by: Michal Privoznik > --- > src/util/virrandom.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/util/virrandom.c b/src/util/virrandom.c > index 01cc82a052..3c011a8615 100644 > --- a/src/util/virrandom.c > +++ b/src/util/virrandom.c > @@ -68,7 +68,7 @@ uint64_t virRandomBits(int nbits) > return 0; > } > > -ret &= (1U << nbits) - 1; > +ret &= (1ULL << nbits) - 1; > return ret; > } Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 4.6.0] tests: qemucapsprobe: Fix output after swithching to jansson
On 07/30/2018 04:59 PM, Peter Krempa wrote:
> Jansson does not put a newline at the end of formatted JSON strings.
> This breaks the qemucapsprobe utility as we need to keep the spacing so
> that tests work. Add an explicit newline.
>
> Signed-off-by: Peter Krempa
> ---
> tests/qemucapsprobemock.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/qemucapsprobemock.c b/tests/qemucapsprobemock.c
> index 5936975505..049b16273a 100644
> --- a/tests/qemucapsprobemock.c
> +++ b/tests/qemucapsprobemock.c
> @@ -76,6 +76,7 @@ qemuMonitorSend(qemuMonitorPtr mon,
> printLineSkipEmpty("\n", stdout);
>
> printLineSkipEmpty(reformatted, stdout);
> +printLineSkipEmpty("\n", stdout);
> VIR_FREE(reformatted);
>
> return realQemuMonitorSend(mon, msg);
> @@ -116,6 +117,7 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon,
> printLineSkipEmpty("\n", stdout);
>
> printLineSkipEmpty(json, stdout);
> +printLineSkipEmpty("\n", stdout);
> }
>
> cleanup:
>
ACK and SFF.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] viriscsi: Request more random bits for interface name
In virStorageBackendCreateIfaceIQN() the virRandomBits() is
called in order to use random bits to generate random name for
new interface. However, virAsprintf() is expecting 32 bits and we
are requesting only 30.
Signed-off-by: Michal Privoznik
---
src/util/viriscsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index 653b4fd932..f00aeb53a7 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -221,7 +221,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
if (virAsprintf(_ifacename,
"libvirt-iface-%08llx",
-(unsigned long long)virRandomBits(30)) < 0)
+(unsigned long long)virRandomBits(32)) < 0)
return -1;
VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'",
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] util: Don't overflow in virRandomBits
The function is supposed to return up to 64bit long integer. In order to do that it calls virRandomBytes() to fill the integer with random bytes and then masks out everything but requested bits. However, when doing that it shifts 1U and not 1ULL. So effectively, requesting 32 random bis or more always return 0 which is not random enough. Signed-off-by: Michal Privoznik --- src/util/virrandom.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virrandom.c b/src/util/virrandom.c index 01cc82a052..3c011a8615 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -68,7 +68,7 @@ uint64_t virRandomBits(int nbits) return 0; } -ret &= (1U << nbits) - 1; +ret &= (1ULL << nbits) - 1; return ret; } -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for v4.6.0 0/2] Fix some random problems
Bad pun. But while debugging the issue Bjoern raised [1] I've noticed these problems. https://www.redhat.com/archives/libvir-list/2018-July/msg02101.html Michal Prívozník (2): util: Don't overflow in virRandomBits viriscsi: Request more random bits for interface name src/util/viriscsi.c | 2 +- src/util/virrandom.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 4.6.0] tests: qemucapsprobe: Fix output after swithching to jansson
On Mon, Jul 30, 2018 at 16:59:34 +0200, Peter Krempa wrote: > Jansson does not put a newline at the end of formatted JSON strings. > This breaks the qemucapsprobe utility as we need to keep the spacing so > that tests work. Add an explicit newline. > > Signed-off-by: Peter Krempa > --- > tests/qemucapsprobemock.c | 2 ++ > 1 file changed, 2 insertions(+) Since we are going ahead with using jansson in libvirt this patch should be included in the release for completness. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of libvirt-4.6.0 Release Candidate 2
On Tue, Jul 31, 2018 at 03:54:43PM +0200, Bjoern Walk wrote: > Bjoern Walk [2018-07-31, 03:16PM +0200]: > > I have not yet had the time to figure out what goes wrong, any ideas are > > welcome. > > Ah, classic. The mocked virRandomBytes function is not endian-agnostic, > generating a different interface name as expected by the test. I'm not seeing why virRandomBytes is affected by endian-ness. It is simply populating an array of bytes, so there's no endin issues to consider there. Can you elaborate on the actual error messages you are getting from the tests, and what aspect makes you think virRandomBytes is the problem ? Seems more likely that it is something higher up the call stack. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of libvirt-4.6.0 Release Candidate 2
On Wed, Aug 01, 2018 at 12:40:14 +0200, Michal Privoznik wrote: > On 07/31/2018 07:19 PM, Ján Tomko wrote: > > And I'm still unsure about leaving in > > commit 55ce65646348884656fd7bf3f109ebf8f7603494 > > qemu: Use the correct vm def on cold attach > > https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=55ce6564634 > > Which means attach-device --live --config will attach an interface > > with a different MAC address in live and persistent definition. Laine? > > > > Well, It's not only MAC address that can change. The device address > might change too. This points to a broader problem. When we are parsing > a device XML we fill in the blanks in postParse callbacks. However, > those look only at either live or at inactive XML. Not at both at the > same time. So how can we fill in the blanks that would be valid for both > XMLs? We can fill in the definition and then copy it and validate it afterwards. That way the blanks are filled and we can then validate that it fits into the other definition. The description here sounds that in this release we made things worse than it was before though. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: qemuDomainChangeNet: validity checks should be done before XML autocompletion
We need to first perform the checks for changed/missing elements
and then we can overwrite the missing ones. Otherwise we might
falsely report successfull update, because some elements got
overwritten before the validity checks.
https://bugzilla.redhat.com/show_bug.cgi?id=1599513
Signed-off-by: Katerina Koukiou
---
src/qemu/qemu_hotplug.c | 43 ++---
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1488f0a7c2..8d98c149e2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3445,23 +3445,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
goto cleanup;
}
-/* info: if newdev->info is empty, fill it in from olddev,
- * otherwise verify that it matches - nothing is allowed to
- * change. (There is no helper function to do this, so
- * individually check the few feidls of virDomainDeviceInfo that
- * are relevant in this case).
+/* info: Nothing is allowed to change. First check for changes and
+ * then fill the missing newdev->info from olddev.
*/
-if (!virDomainDeviceAddressIsValid(>info,
- VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
-virDomainDeviceInfoCopy(>info, >info) < 0) {
-goto cleanup;
-}
-if (!virPCIDeviceAddressEqual(>info.addr.pci,
- >info.addr.pci)) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot modify network device guest PCI address"));
-goto cleanup;
-}
/* grab alias from olddev if not set in newdev */
if (!newdev->info.alias &&
VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0)
@@ -3469,26 +3455,51 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
/* device alias is checked already in virDomainDefCompatibleDevice */
+if (newdev->info.rombar == VIR_TRISTATE_BOOL_ABSENT)
+newdev->info.rombar = olddev->info.rombar;
if (olddev->info.rombar != newdev->info.rombar) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom bar setting"));
goto cleanup;
}
+
+if (!newdev->info.romfile &&
+VIR_STRDUP(newdev->info.romfile, olddev->info.romfile) < 0)
+goto cleanup;
if (STRNEQ_NULLABLE(olddev->info.romfile, newdev->info.romfile)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network rom file"));
goto cleanup;
}
+
+if (newdev->info.bootIndex == 0)
+newdev->info.bootIndex = olddev->info.bootIndex;
if (olddev->info.bootIndex != newdev->info.bootIndex) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device boot index setting"));
goto cleanup;
}
+
+if (newdev->info.romenabled == VIR_TRISTATE_BOOL_ABSENT)
+newdev->info.romenabled = olddev->info.romenabled;
if (olddev->info.romenabled != newdev->info.romenabled) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot modify network device rom enabled setting"));
goto cleanup;
}
+
+/* if pci addr is missing or is invalid we overwrite all info struct */
+if (!virDomainDeviceAddressIsValid(>info,
+ VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+virDomainDeviceInfoCopy(>info, >info) < 0) {
+goto cleanup;
+}
+if (!virPCIDeviceAddressEqual(>info.addr.pci,
+ >info.addr.pci)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot modify network device guest PCI address"));
+goto cleanup;
+}
/* (end of device info checks) */
if (STRNEQ_NULLABLE(olddev->filter, newdev->filter) ||
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dockerfiles PATCH] Update OS when building images
The corresponding libvirt-jenkins-ci commit is dcbad35f45fe.
Signed-off-by: Andrea Bolognani
---
Pushed under the Dockerfile refresh rule.
buildenv-centos-7.Dockerfile | 3 ++-
buildenv-debian-8.Dockerfile | 1 +
buildenv-debian-9.Dockerfile | 1 +
buildenv-debian-sid.Dockerfile | 1 +
buildenv-fedora-27.Dockerfile | 3 ++-
buildenv-fedora-28.Dockerfile | 3 ++-
buildenv-fedora-rawhide.Dockerfile | 3 ++-
buildenv-ubuntu-16.Dockerfile | 1 +
buildenv-ubuntu-18.Dockerfile | 1 +
9 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/buildenv-centos-7.Dockerfile b/buildenv-centos-7.Dockerfile
index 2c4c26d..896bf68 100644
--- a/buildenv-centos-7.Dockerfile
+++ b/buildenv-centos-7.Dockerfile
@@ -65,6 +65,7 @@ ENV PACKAGES audit-libs-devel \
sudo \
systemtap-sdt-devel \
vim
-RUN yum install -y ${PACKAGES} && \
+RUN yum update -y && \
+yum install -y ${PACKAGES} && \
yum autoremove -y && \
yum clean all -y
diff --git a/buildenv-debian-8.Dockerfile b/buildenv-debian-8.Dockerfile
index 8a8c3b0..8877528 100644
--- a/buildenv-debian-8.Dockerfile
+++ b/buildenv-debian-8.Dockerfile
@@ -71,6 +71,7 @@ ENV PACKAGES augeas-tools \
xsltproc \
zfs-fuse
RUN apt-get update && \
+apt-get dist-upgrade -y && \
apt-get install -y ${PACKAGES} && \
apt-get autoremove -y && \
apt-get autoclean -y
diff --git a/buildenv-debian-9.Dockerfile b/buildenv-debian-9.Dockerfile
index c3af421..0592c97 100644
--- a/buildenv-debian-9.Dockerfile
+++ b/buildenv-debian-9.Dockerfile
@@ -73,6 +73,7 @@ ENV PACKAGES augeas-tools \
xsltproc \
zfs-fuse
RUN apt-get update && \
+apt-get dist-upgrade -y && \
apt-get install -y ${PACKAGES} && \
apt-get autoremove -y && \
apt-get autoclean -y
diff --git a/buildenv-debian-sid.Dockerfile b/buildenv-debian-sid.Dockerfile
index a922457..2d0310d 100644
--- a/buildenv-debian-sid.Dockerfile
+++ b/buildenv-debian-sid.Dockerfile
@@ -73,6 +73,7 @@ ENV PACKAGES augeas-tools \
xsltproc \
zfs-fuse
RUN apt-get update && \
+apt-get dist-upgrade -y && \
apt-get install -y ${PACKAGES} && \
apt-get autoremove -y && \
apt-get autoclean -y
diff --git a/buildenv-fedora-27.Dockerfile b/buildenv-fedora-27.Dockerfile
index 2f35a2e..09ec721 100644
--- a/buildenv-fedora-27.Dockerfile
+++ b/buildenv-fedora-27.Dockerfile
@@ -73,6 +73,7 @@ ENV PACKAGES audit-libs-devel \
wireshark-devel \
xen-devel \
zfs-fuse
-RUN yum install -y ${PACKAGES} && \
+RUN yum update -y && \
+yum install -y ${PACKAGES} && \
yum autoremove -y && \
yum clean all -y
diff --git a/buildenv-fedora-28.Dockerfile b/buildenv-fedora-28.Dockerfile
index f4f7198..60afba5 100644
--- a/buildenv-fedora-28.Dockerfile
+++ b/buildenv-fedora-28.Dockerfile
@@ -73,6 +73,7 @@ ENV PACKAGES audit-libs-devel \
wireshark-devel \
xen-devel \
zfs-fuse
-RUN yum install -y ${PACKAGES} && \
+RUN yum update -y && \
+yum install -y ${PACKAGES} && \
yum autoremove -y && \
yum clean all -y
diff --git a/buildenv-fedora-rawhide.Dockerfile
b/buildenv-fedora-rawhide.Dockerfile
index 189ea59..33b47e1 100644
--- a/buildenv-fedora-rawhide.Dockerfile
+++ b/buildenv-fedora-rawhide.Dockerfile
@@ -97,6 +97,7 @@ ENV PACKAGES audit-libs-devel \
wireshark-devel \
xen-devel \
zfs-fuse
-RUN yum install -y ${PACKAGES} && \
+RUN yum update -y && \
+yum install -y ${PACKAGES} && \
yum autoremove -y && \
yum clean all -y
diff --git a/buildenv-ubuntu-16.Dockerfile b/buildenv-ubuntu-16.Dockerfile
index bc387a1..c904c6e 100644
--- a/buildenv-ubuntu-16.Dockerfile
+++ b/buildenv-ubuntu-16.Dockerfile
@@ -74,6 +74,7 @@ ENV PACKAGES augeas-tools \
xsltproc \
zfs-fuse
RUN apt-get update && \
+apt-get dist-upgrade -y && \
apt-get install -y ${PACKAGES} && \
apt-get autoremove -y && \
apt-get autoclean -y
diff --git a/buildenv-ubuntu-18.Dockerfile b/buildenv-ubuntu-18.Dockerfile
index 2a15f14..bbd32c1 100644
--- a/buildenv-ubuntu-18.Dockerfile
+++ b/buildenv-ubuntu-18.Dockerfile
@@ -74,6 +74,7 @@ ENV PACKAGES augeas-tools \
xsltproc \
zfs-fuse
RUN apt-get update && \
+apt-get dist-upgrade -y && \
apt-get install -y ${PACKAGES} && \
apt-get autoremove -y && \
apt-get autoclean -y
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] conf: virDomainDefValidateInternal prohibit some characters in shmem name
On Fri, 2018-07-27 at 13:19 -0400, John Ferlan wrote:
> You shouldn't drop libvir-list when you reply...
Sorry, I dropped it by mistake.
>
> On 07/27/2018 06:35 AM, skob...@redhat.com wrote:
> > On Mon, 2018-07-23 at 17:32 -0400, John Ferlan wrote:
> > >
> > > On 07/12/2018 09:10 AM, Simon Kobyda wrote:
> > > > XML shmem name will not include character '/', and will not be
> > > > equal to strings
> > > > "." or "..", as shmem name is used in a path.
> > >
> > > Validate that the provided XML shmem name is not directory
> > > specific
> > > "."
> > > or ".." names as well as ensuring that there is no path separator
> > > '/'
> > > in
> > > the name.
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1192400
> > > > ---
> > > >
> > > > Changes in V2
> > > > - Added error reports
> > > > - Error situation will happen only if shmem name is
> > > > equal to
> > > > "." or "..", however their occurence in a name
> > > > compromised of
> > > > more
> > > > characters is allowed.
> > > >
> > > > src/conf/domain_conf.c | 22 ++
> > > > 1 file changed, 22 insertions(+)
> > > >
> > >
> > > I believe this actually belongs in
> > > virDomainDeviceDefValidateInternal
> > > for case VIR_DOMAIN_DEVICE_SHMEM.
> > > Also, should the docs/schemas/domaincommon.rng be modified?
> > > Currently
> > > it
> > > has:
> > >
> > >
> > >
> > >
> > >
> > > [^/]*
> > >
> > >
> >
> > Is it a good idea? To create such regular expression, I believe it
> > would make it unreadable for another person, ergo useless. I don't
> > want
> > it to end up as IPv6. I've benn told that actual parser can be, and
> > sometimes is stricter than scheme.
> >
>
> Well this is what I was hoping others would be able to chime in on.
> Hence why you ask on list. I don't disagree that being stricter in
> Parse
> is fine. Doing regexes is like reading a foreign language to me at
> times. Some just don't make sense.
Yeah, that's why I don't want to try to insert complicated regex in
docs/schemas/domaincommon.rng.
>
> I think that regex is indicating - any character except '/', but I'm
> not
> sure. If it is, it's ironic that we have to check for '/'
> specifically
> since it shouldn't be passing the xml validation test - OK at least
> if
> one was editing via virsh.
>
> John
>
You are right about what that regex does. But from what I heard, that
functionality is rather new to libvirt, so users have option to bypass
that verification. So I would like to also leave it in code in case
users try to bypass that regex verification.
Simon.
> > > Consider how other names are limited in their scope. The
> > > basictypes.rng
> > > has a number of examples.
> > >
> > > Naturally, the problem with changing it is that someone somewhere
> > > will
> > > complain, but libvirt used to accept this other format. Right now
> > > I
> > > would think the scope a bit too broad.
> >
> > But then we cannot fix most of the bugs, since there could always
> > be
> > some user which relies on bug behavior.
> > >
> > > If we are to limit the name we should also document in
> > > docs/formatdomain.html.in that the shmem name is "limited" in
> > > name to
> > > avoid the '/' character, ".", and "..".
> > >
> > > BTW: My regex isn't that good, but it would seem '/' is an
> > > invalid
> > > character by XML standards even though the code never checked for
> > > it.
> > > Using virt-xml-validate would "validate" whether
> > > someone
> > > provides valid XML.
> > >
> > >
> > > John
> > >
> > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > > index 7ab2953d83..6b34c17de4 100644
> > > > --- a/src/conf/domain_conf.c
> > > > +++ b/src/conf/domain_conf.c
> > > > @@ -6107,6 +6107,8 @@ virDomainDefLifecycleActionValidate(const
> > > > virDomainDef *def)
> > > > static int
> > > > virDomainDefValidateInternal(const virDomainDef *def)
> > > > {
> > > > +size_t i;
> > > > +
> > > > if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
> > > > return -1;
> > > >
> > > > @@ -6136,6 +6138,26 @@ virDomainDefValidateInternal(const
> > > > virDomainDef *def)
> > > > return -1;
> > > > }
> > > >
> > > > +for (i = 0; i < def->nshmems; i++) {
> > > > +if (strchr(def->shmems[i]->name, '/')) {
> > > > +virReportError(VIR_ERR_XML_ERROR, "%s",
> > > > + _("shmem name cannot include '/'
> > > > character"));
> > > > +return -1;
> > > > +}
> > > > +
> > > > +if (STREQ(def->shmems[i]->name, ".")) {
> > > > +virReportError(VIR_ERR_XML_ERROR, "%s",
> > > > + _("shmem name cannot be equal to
> > > > '.'"));
> > > > +return -1;
> > > > +}
> > > > +
> > > > +if (STREQ(def->shmems[i]->name, "..")) {
> > > > +virReportError(VIR_ERR_XML_ERROR, "%s",
> > > > +
Re: [libvirt] Matching the type of mediated devices in the migration
Hi: Let me summarize the understanding so far I got from the discussions since I am new to this discussion. The mdev_type would be a generic stuff since we don't want userspace application to be confused. The example of mdev_type is: There are several pre-defined mdev_types with different configurations, let's say MDEV_TYPE A/B/C. The HW 1.0 might only support MDEV_TYPE A, the HW 2.0 might support both MDEV_TYPE A and B, but due to HW difference, we cannot migrate MDEV_TYPE A with HW 1.0 to MDEV_TYPE A with HW 2.0 even they have the same MDEV_TYPE. So we need a device version either in the existing MDEV_TYPE or a new sysfs entry. Libvirt would have to check MDEV_TYPE match between source machine and destination machine, then the device version. If any of them is different, then it fails the migration. If my above understanding is correct, for VFIO part, we could define the device version as string or a magic number. For example, the vendor mdev driver could pass the vendor/device id and a version to VFIO and VFIO could expose them in the UUID sysfs no matter through a new sysfs entry or through existing MDEV_TYPE. I prefer to expose it in the mdev_supported_types, since the libvirt node device list could extract the device version when it enumerating the host PCI devices or other devices, which supports mdev. We can also put it into UUID sysfs, but the user might have to first logon the target machine and then check the UUID and the device version by themselves, based on current code of libvirty. I suppose all the host device management would be in node device in libvirt, which provides remotely management of the host devices. For the format of a device version, an example would be: Vendor ID(16bit)Device ID(16bit)Class ID(16bit)Version(16bit) For string version of the device version, I guess we have to define the max string length, which is hard to say yet. Also, a magic number is easier to be put into the state data header during the migration. Thanks, Zhi. -Original Message- From: Alex Williamson [mailto:alex.william...@redhat.com] Sent: Tuesday, July 31, 2018 12:49 AM To: Wang, Zhi A Cc: libvir-list@redhat.com; kwankh...@nvidia.com Subject: Re: Matching the type of mediated devices in the migration On Tue, 31 Jul 2018 04:05:11 +0800 Zhi Wang wrote: > On 07/30/18 23:56, Alex Williamson wrote: > > On Sun, 29 Jul 2018 21:19:41 + > > "Wang, Zhi A" wrote: > > > >> BACKGROUND > >> > >> As the live migration of mdev is going to be supported in VFIO, a scheme > >> of deciding if a mdev could be migratable between the source machine and > >> the destination machine is needed. Mostly, this email is going to discuss > >> a possible solution which needs fewer modifications of libvirt/VFIO. > >> > >> The configuration of a mdev is located in the domain XML, which guides > >> libvirt how to find the mdev and generating the command line for QEMU. It > >> basically only includes the UUID of a mdev. The domain XML of the source > >> machine and destination machine are going to be compared before the > >> migration really happens. Each configuration item would be compared and > >> checked by libvirt. If one item of the source machine is different from > >> the item of destination machine, the migration fails. For mdev, there is > >> no any check/match before the migration happens yet. > >> > >> The user could use the node device list of libvirt to list the host > >> devices and see the capabilities of those devices. The current node device > >> code of libvirt has already been able to extract the supported mdev types > >> from a host PCI device, plus some basic information, like max supported > >> mdev instance of a host PCI device. > >> > >> THE SOLUTION > >> > >> To strictly check the mdev type and make sure the migration happens > >> between the compatible mediated devices, three new mandatory elements in > >> the domain XML below the hostdev element would be introduced: > >> > >> vendorid: The vendor ID of the mdev, which comes from the host PCI device. > >> A user could obtain this information from the host PCI device which > >> supports mdev in the node device list. > >> productid: The product ID of the mdev, which also comes from the host PCI > >> device. A user could obtain this information from the same approach above. > >> > > > > The parent of an mdev device is not necessarily a PCI device. > Good point. I didn't get that. > > > >> mdevtype: The type of the mdev. As the creation of the mdev is managed by > >> the user, the user knows the type of the mdev and would be responsible for > >> filling out this information. > >> > >> These three elements are only needed when the device API of a mdev is > >> "vfio-PCI". Take the example of mdev configuration from > >> https://libvirt.org/formatdomain.html to illustrate the modification: > >> > >> > >> > >> > >> > >>0xdead > >>0xbeef > >>
Re: [libvirt] [Qemu-devel] [PATCH for-3.1] s390x: remove 's390-squash-mcss' option
On Tue, 07/24 11:24, Cornelia Huck wrote: > diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c > index a02d708239..1bd6c8b458 100644 > --- a/hw/s390x/css-bridge.c > +++ b/hw/s390x/css-bridge.c > @@ -106,7 +106,6 @@ VirtualCssBus *virtual_css_bus_init(void) > /* Create bus on bridge device */ > bus = qbus_create(TYPE_VIRTUAL_CSS_BUS, dev, "virtual-css"); > cbus = VIRTUAL_CSS_BUS(bus); Not used now? Fam -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: qemuDomainChangeNet: don't overwrite device info when pci addr is missing
On Tue, Jul 31, 2018 at 07:24:00PM +0200, Ján Tomko wrote: > On Tue, Jul 31, 2018 at 05:32:51PM +0200, Katerina Koukiou wrote: > > On Tue, Jul 31, 2018 at 05:15:51PM +0200, Ján Tomko wrote: > > > On Tue, Jul 31, 2018 at 04:34:39PM +0200, Katerina Koukiou wrote: > > > > When trying to update an interface's rom settings with an device XML > > > > that is missing the PCI addr element, all new rom settings where not > > > > applied. > > > > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1599513 > > > > > > > > Signed-off-by: Katerina Koukiou > > > > --- > > > > Not sure why we chose to overwrite the whole info before though, I hope > > > > that this doesn't cause side problems. > > > > > > > > > > It allows the user to omit parts of the XML that do not need changing. > > > > But this happens now only if PCI addr is missing. > > > > > > > > By dropping the virDomainDeviceInfoCopy call, we start validating the > > > attributes that weren't provided in the XML. > > > > > > If we want to keep this convenience functionality, we should also autofill > > > other fields of virDomainDeviceInfo which weren't provided in the XML. > > > > The functionality right now is that only if PCI addr is missing, then > > autofilling happens, by overwriting all other passed params. I don't > > really consider it a convenience functionality, more of confusing. > > It is both confusing and convenient. > > > I think overwritting the specific object that is missing is more > > correct. What do you think? > > Well I can at least perform the existing checks for the info before they get overwritten. I 'll repost with this change. Katerina > > Yes. I'm afraid we cannot get rid of the 'change with incomplete XML' > behavior after all the years. > > Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: avoid symbol clash between json libraries
On Wed, Aug 01, 2018 at 09:31:49AM +0200, Peter Krempa wrote:
> On Tue, Jul 31, 2018 at 15:55:28 +0100, Daniel Berrange wrote:
> > The jansson and json-glib libraries both export symbols with a json_
> > name prefix and json_object_iter_next() clashes between them.
> >
> > Unfortunately json_glib is linked in by GTK, so any app using GTK and
> > libvirt will get a clash, resulting in SEGV. This also affects the NSS
> > module provided by libvirt
> >
> > Instead of directly linking to jansson, use dlopen() with the RTLD_LOCAL
> > flag which allows us to hide the symbols from the application that loads
> > libvirt or the NSS module.
> >
> > Some preprocessor black magic and wrapper functions are used to redirect
> > calls into the dlopen resolved symbols.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > libvirt.spec.in | 2 +
> > src/Makefile.am | 3 +
> > src/util/Makefile.inc.am | 3 +-
> > src/util/virjson.c | 9 +-
> > src/util/virjsoncompat.c | 253 +++
> > src/util/virjsoncompat.h | 86 +
> > 6 files changed, 354 insertions(+), 2 deletions(-)
> > create mode 100644 src/util/virjsoncompat.c
> > create mode 100644 src/util/virjsoncompat.h
>
> [...]
>
> > diff --git a/src/util/virjson.c b/src/util/virjson.c
> > index 01a387b2f7..5bab662cd3 100644
> > --- a/src/util/virjson.c
> > +++ b/src/util/virjson.c
> > @@ -1437,7 +1437,8 @@ virJSONValueCopy(const virJSONValue *in)
> >
> >
> > #if WITH_JANSSON
> > -# include
> > +
> > +# include "virjsoncompat.h"
> >
> > static virJSONValuePtr
> > virJSONValueFromJansson(json_t *json)
> > @@ -1524,6 +1525,9 @@ virJSONValueFromString(const char *jsonstring)
> > size_t flags = JSON_REJECT_DUPLICATES |
> > JSON_DECODE_ANY;
> >
> > +if (virJSONInitialize() < 0)
> > +return NULL;
> > +
> > if (!(json = json_loads(jsonstring, flags, ))) {
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > _("failed to parse JSON %d:%d: %s"),
> > @@ -1630,6 +1634,9 @@ virJSONValueToString(virJSONValuePtr object,
> > json_t *json;
> > char *str = NULL;
> >
> > +if (virJSONInitialize() < 0)
> > +return NULL;
> > +
> > if (pretty)
> > flags |= JSON_INDENT(2);
> > else
>
> [Note that some mails did not make it to the list so I don't know if
> this issue was raised.]
>
> If we initialize this when doing JSON operations and the initialization
> fails we'll end up with libvirtd in a crippled state not able to use
> anything in qemu which is in my opinion VERY bad.
>
> In addition it also kills all running VMs when reconnecting since
> monitor will just not work.
>
> I think libvirtd should terminate right away if we can't initialize this
> shim rather than execute some other code.
We can do that by calling virJSONInitialize from libvirtd main. It should
never fail unless someone instaled libvirtd without jansson being present,
or if jansson broke its API.
> Additionally, why is this even necessary for libvirtd? Not that it would
> ever link with any clashing libraries.
It may not currently be neccessary for libvirtd, but I can't guarantee
that's always going to be the case. We link to so many libraries, we can
be surprised at anytime with something pulling in a clashing json impl.
The json-c library also includes symbols with a json_* prefix in their
name, so that's a 3d json impl which might clash.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: monitor: Fix incrementing of 'nstats' in qemuMonitorJSONBlockStatsCollectData
On Wed, Aug 01, 2018 at 09:24:29AM +0200, Peter Krempa wrote: commit 8d9ca6cdb3a5 refactored qemuMonitorJSONBlockStatsCollectData so that the number of stats is passed back via a pointer. The commit failed to fix the macro which increments the number of stats to increment the actual pointee. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: monitor: Fix incrementing of 'nstats' in qemuMonitorJSONBlockStatsCollectData
commit 8d9ca6cdb3a5 refactored qemuMonitorJSONBlockStatsCollectData so
that the number of stats is passed back via a pointer. The commit failed
to fix the macro which increments the number of stats to increment the
actual pointee.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor_json.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9acf62e0bb..2921f110a9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2299,7 +2299,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev,
#define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \
if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \
-nstats++; \
+(*nstats)++; \
if (virJSONValueObjectGetNumberLong(stats, NAME, ) < 0) { \
virReportError(VIR_ERR_INTERNAL_ERROR, \
_("cannot read %s statistic"), NAME); \
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Availability of libvirt-4.6.0 Release Candidate 2
On Tue, Jul 31, 2018 at 19:19:33 +0200, Ján Tomko wrote: > On Tue, Jul 31, 2018 at 12:19:28PM +0800, Daniel Veillard wrote: > > It is out, tagged in git and with signed tarball and rpms at the > > usual place: > > > > ftp://libvirt.org/libvirt/ > > > > in my limited testing it works but we have that pending issue > > raised by Andrea. worse case someone reverse the patches and > > allows to build against the old lib. > > > > Please give it some testing, I will watch the commits and > > if there isn't any solution 2 days from now I will postpone the > > GA by a couple of days. Hopefully that won't be needed :-) > > > > Since commit 50edca1331298bfcb2622e8fe588d493aff9ab68 >qemu: monitor: Add the 'query-nodes' argument for query-blockstats > https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=50edca133129 > > 'virsh domblkstat' shows nothing for me with QEMU 2.9.0 (works with > v3.0.0-rc0-66-gccf02d73d1) This is probably a red herring. The 'B' modifier used in the patch does not put the argument unless it's true to the monitor. I traced the problem to a bug in commit 8d9ca6cdb3a58414 where I've changed 'nstats' to a pointer but did not fix the usage in the macro which gathers the stats. This means that the expanded code was incrementing the pointer which was not dereferenced aferwards rather than the value itself. I'll post a patch soon. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
