Re: [libvirt] [PATCH 1/4] cpu: allow include files for CPU definition
On Tue, Aug 14, 2018 at 12:55:19PM +0200, Jiri Denemark wrote:
> On Wed, Aug 01, 2018 at 18:02:29 +0100, Daniel P. Berrangé wrote:
> > Allow for syntax
> >
> >
>
> It seems the code should just work with
>
>
>
> but Makefile.am and libvirt.spec would need some adjustment.
>
> > 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,
> ..
> > +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");
>
> This would be a nice candidate for VIR_AUTOFREE(char *) :-)
Since none of the src/cpu code has been switched to this style yet, its
probably best not to introduce it here. Rather wait for the bulk convert
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/4] cpu: allow include files for CPU definition
On Wed, Aug 01, 2018 at 18:02:29 +0100, Daniel P. Berrangé wrote:
> Allow for syntax
>
>
It seems the code should just work with
but Makefile.am and libvirt.spec would need some adjustment.
> 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,
..
> +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");
This would be a nice candidate for VIR_AUTOFREE(char *) :-)
> +VIR_DEBUG("Finding CPU map include '%s'", filename);
> +if (cpuMapLoadInclude(filename, callback, data) < 0) {
> +VIR_FREE(filename);
> +goto cleanup;
> +}
> +VIR_FREE(filename);
> +}
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] cpu: allow include files for CPU definition
On 08/01/2018 01:02 PM, Daniel P. Berrangé wrote:
> 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(-)
>
I'll assume you got the spec/makefile magic correct as it's not in my
wheelhouse!
> 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 loadIncludes(xmlXPathContextPtr ctxt,
> +cpuMapLoadCallback callback,
> +void *data)
static int
loadIncludes(...)
for consistency
> +{
> +int ret = -1;
[...]
>
> 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);
Considering the subsequent NULL check:
s/arch/NULLSTR(arch)/
or move the VIR_DEBUG after the check (IDC).
I'm not even sure why @mapfile filling is above the argument validation
checks, but that's a different issue and since more changes are about to
come, it's not that important ;-)... As long there is either a NULLSTR
or moved message, that's fine.
Reviewed-by: John Ferlan
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
