Markus Groß
2011-May-23 11:42 UTC
[Xen-devel] libxc: correctly unmap pages after core-dumping a pv guest
Hi,
while implementing core dumping functionality for the libxl driver
of libvirt, I discovered an issue with mapping pages of a pv guest.
After dumping the core of a pv guest the domain was not cleared up
properly and some pages were not unmapped. This issue is similar
to the one reported here:
http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01314.html
In xc_domain_dumpcore_via_callback in the file xc_core.c the function
xc_core_arch_map_p2m is called to map P2M_FL_ENTRIES pages to the variable p2m.
But to unmap the pages later, the dinfo->p2m_size has to be set accordingly.
This was not done, instead a variable named p2m_size was set.
This way P2M_FL_ENTRIES was always zero and the pages were left mapped.
The following patch fixes this.
Best regards,
Markus
diff -r 7c7ef1b6f4e5 tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c Tue Apr 26 14:11:18 2011 +0100
+++ b/tools/libxc/xc_core.c Mon May 23 13:36:23 2011 +0200
@@ -468,7 +468,6 @@
int auto_translated_physmap;
xen_pfn_t *p2m = NULL;
- unsigned long p2m_size = 0;
struct xen_dumpcore_p2m *p2m_array = NULL;
uint64_t *pfn_array = NULL;
@@ -569,7 +568,7 @@
}
sts = xc_core_arch_map_p2m(xch, dinfo->guest_width, &info,
live_shinfo,
- &p2m, &p2m_size);
+ &p2m, &dinfo->p2m_size);
if ( sts != 0 )
goto out;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Markus Groß
2011-May-23 12:24 UTC
Re: [Xen-devel] libxc: correctly unmap pages after core-dumping a pv guest
I forgot to put [PATCH] in the subject, sorry about that. Am Montag 23 Mai 2011 13:42:28 schrieb Markus Groß:> Hi, > > while implementing core dumping functionality for the libxl driver > of libvirt, I discovered an issue with mapping pages of a pv guest. > > After dumping the core of a pv guest the domain was not cleared up > properly and some pages were not unmapped. This issue is similar > to the one reported here: > http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01314.html > > In xc_domain_dumpcore_via_callback in the file xc_core.c the function > xc_core_arch_map_p2m is called to map P2M_FL_ENTRIES pages to the variable p2m. > But to unmap the pages later, the dinfo->p2m_size has to be set accordingly. > This was not done, instead a variable named p2m_size was set. > This way P2M_FL_ENTRIES was always zero and the pages were left mapped. > > The following patch fixes this. > > Best regards, > Markus > > diff -r 7c7ef1b6f4e5 tools/libxc/xc_core.c > --- a/tools/libxc/xc_core.c Tue Apr 26 14:11:18 2011 +0100 > +++ b/tools/libxc/xc_core.c Mon May 23 13:36:23 2011 +0200 > @@ -468,7 +468,6 @@ > > int auto_translated_physmap; > xen_pfn_t *p2m = NULL; > - unsigned long p2m_size = 0; > struct xen_dumpcore_p2m *p2m_array = NULL; > > uint64_t *pfn_array = NULL; > @@ -569,7 +568,7 @@ > } > > sts = xc_core_arch_map_p2m(xch, dinfo->guest_width, &info, live_shinfo, > - &p2m, &p2m_size); > + &p2m, &dinfo->p2m_size); > if ( sts != 0 ) > goto out; > } > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-May-23 12:46 UTC
Re: [Xen-devel] libxc: correctly unmap pages after core-dumping a pv guest
On Mon, 2011-05-23 at 13:24 +0100, Markus Groß wrote:> I forgot to put [PATCH] in the subject, sorry about that.That doesn''t matter too much (it just increases the chance of attracting a maintainers attention). What does matter though is that we need your Signed-off-by per the DCO, [0]. Please can you resend with that? Thanks! Ian. [0] http://lwn.net/Articles/437739/> > Am Montag 23 Mai 2011 13:42:28 schrieb Markus Groß: > > Hi, > > > > while implementing core dumping functionality for the libxl driver > > of libvirt, I discovered an issue with mapping pages of a pv guest. > > > > After dumping the core of a pv guest the domain was not cleared up > > properly and some pages were not unmapped. This issue is similar > > to the one reported here: > > http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01314.html > > > > In xc_domain_dumpcore_via_callback in the file xc_core.c the function > > xc_core_arch_map_p2m is called to map P2M_FL_ENTRIES pages to the variable p2m. > > But to unmap the pages later, the dinfo->p2m_size has to be set accordingly. > > This was not done, instead a variable named p2m_size was set. > > This way P2M_FL_ENTRIES was always zero and the pages were left mapped. > > > > The following patch fixes this. > > > > Best regards, > > Markus > > > > diff -r 7c7ef1b6f4e5 tools/libxc/xc_core.c > > --- a/tools/libxc/xc_core.c Tue Apr 26 14:11:18 2011 +0100 > > +++ b/tools/libxc/xc_core.c Mon May 23 13:36:23 2011 +0200 > > @@ -468,7 +468,6 @@ > > > > int auto_translated_physmap; > > xen_pfn_t *p2m = NULL; > > - unsigned long p2m_size = 0; > > struct xen_dumpcore_p2m *p2m_array = NULL; > > > > uint64_t *pfn_array = NULL; > > @@ -569,7 +568,7 @@ > > } > > > > sts = xc_core_arch_map_p2m(xch, dinfo->guest_width, &info, live_shinfo, > > - &p2m, &p2m_size); > > + &p2m, &dinfo->p2m_size); > > if ( sts != 0 ) > > goto out; > > } > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xensource.com > > http://lists.xensource.com/xen-devel > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Markus Groß
2011-May-23 13:12 UTC
Re: [Xen-devel] [PATCH] libxc: correctly unmap pages after core-dumping a pv guest
Hi,
while implementing core dumping functionality for the libxl driver
of libvirt, I discovered an issue with mapping pages of a pv guest.
After dumping the core of a pv guest the domain was not cleared up
properly and some pages were not unmapped. This issue is similar
to the one reported here:
http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01314.html
In xc_domain_dumpcore_via_callback in the file xc_core.c the function
xc_core_arch_map_p2m is called to map P2M_FL_ENTRIES pages to the
variable p2m.
But to unmap the pages later, the dinfo->p2m_size has to be set accordingly.
This was not done, instead a variable named p2m_size was set.
This way P2M_FL_ENTRIES was always zero and the pages were left mapped.
The following patch fixes this.
Best regards,
Markus
Signed-off-by: Markus Groß <gross <at> univention.de>
diff -r 7c7ef1b6f4e5 tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c Tue Apr 26 14:11:18 2011 +0100
+++ b/tools/libxc/xc_core.c Mon May 23 13:36:23 2011 +0200
@@ -468,7 +468,6 @@
int auto_translated_physmap;
xen_pfn_t *p2m = NULL;
- unsigned long p2m_size = 0;
struct xen_dumpcore_p2m *p2m_array = NULL;
uint64_t *pfn_array = NULL;
@@ -569,7 +568,7 @@
}
sts = xc_core_arch_map_p2m(xch, dinfo->guest_width, &info,
live_shinfo,
- &p2m, &p2m_size);
+ &p2m, &dinfo->p2m_size);
if ( sts != 0 )
goto out;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2011-May-24 15:07 UTC
Re: [Xen-devel] libxc: correctly unmap pages after core-dumping a pv guest
Markus Groß writes ("[Xen-devel] libxc: correctly unmap pages after
core-dumping a pv guest"):> But to unmap the pages later, the dinfo->p2m_size has to be set
accordingly.
> This was not done, instead a variable named p2m_size was set.
> This way P2M_FL_ENTRIES was always zero and the pages were left mapped.
>
> The following patch fixes this.
Thanks. The existing code here is pretty unfortunate.
In xc_domain_dumpcore_via_callback where you make your change, "dinfo"
refers to a struct domain_info_context which is local to that
function. But in the called function xc_core_arch_map_p2m_rw, "dinfo"
refers to another identical structure allocated locally to the small
wrapper function xc_core_arch_map_p2m. All rather tangled.
But, anyway, your change is correct so I have applied it.
Next time, though, can you please be sure to add a Signed-off-by line,
to signify your certification in accordance with the Developer''s
Certificate of Origin ? In this case I''ll go ahead as it''s
only a
couple of lines.
Thanks,
Ian.
>From Documentation/SubmittingPatches in the Linux kernel tree:
Developer''s Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Markus Groß
2011-May-24 16:22 UTC
Re: [Xen-devel] libxc: correctly unmap pages after core-dumping a pv guest
Quoting Ian Jackson <Ian.Jackson@eu.citrix.com>:> Markus Groß writes ("[Xen-devel] libxc: correctly unmap pages after > core-dumping a pv guest"): >> But to unmap the pages later, the dinfo->p2m_size has to be set accordingly. >> This was not done, instead a variable named p2m_size was set. >> This way P2M_FL_ENTRIES was always zero and the pages were left mapped. >> >> The following patch fixes this. > > Thanks. The existing code here is pretty unfortunate. > > In xc_domain_dumpcore_via_callback where you make your change, "dinfo" > refers to a struct domain_info_context which is local to that > function. But in the called function xc_core_arch_map_p2m_rw, "dinfo" > refers to another identical structure allocated locally to the small > wrapper function xc_core_arch_map_p2m. All rather tangled. > > But, anyway, your change is correct so I have applied it. > > Next time, though, can you please be sure to add a Signed-off-by line, > to signify your certification in accordance with the Developer''s > Certificate of Origin ? In this case I''ll go ahead as it''s only a > couple of lines. > > Thanks, > Ian.Thanks, although I did repost the patch with a Signed-off-by line here: http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01469.html Cheers, Markus> > From Documentation/SubmittingPatches in the Linux kernel tree: > > Developer''s Certificate of Origin 1.1 > > By making a contribution to this project, I certify that: > > (a) The contribution was created in whole or in part by me and I > have the right to submit it under the open source license > indicated in the file; or > > (b) The contribution is based upon previous work that, to the best > of my knowledge, is covered under an appropriate open source > license and I have the right under that license to submit that > work with modifications, whether created in whole or in part > by me, under the same open source license (unless I am > permitted to submit under a different license), as indicated > in the file; or > > (c) The contribution was provided directly to me by some other > person who certified (a), (b) or (c) and I have not modified > it. > > (d) I understand and agree that this project and the contribution > are public and that a record of the contribution (including all > personal information I submit with it, including my sign-off) is > maintained indefinitely and may be redistributed consistent with > this project or the open source license(s) involved. >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-May-24 17:32 UTC
Re: [Xen-devel] libxc: correctly unmap pages after core-dumping a pv guest
Markus Groß writes ("Re: [Xen-devel] libxc: correctly unmap pages after
core-dumping a pv guest"):> Thanks, although I did repost the patch with a Signed-off-by line here:
> http://lists.xensource.com/archives/html/xen-devel/2011-05/msg01469.html
Sorry, I must have missed that in my backlog.
Regards,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel