Mukesh Rathor
2009-Nov-23 23:14 UTC
[Xen-devel] [PATCH] libxc: failure to save/restore 32bit HVM guest
Following appears to be an obvious bug. Save/restore of HVM guests
works fine after the following fix. Including the patch, please let me
know if you agree. I''ll make parallel fix in our previous versions
here.
thanks,
Mukesh
Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
diff -r c8caa281a75a tools/libxc/xc_core.c
--- a/tools/libxc/xc_core.c Mon Nov 23 08:06:54 2009 +0000
+++ b/tools/libxc/xc_core.c Mon Nov 23 15:10:33 2009 -0800
@@ -628,7 +628,7 @@
PERROR("Could not get section header for .xen_prstatus");
goto out;
}
- filesz = sizeof(ctxt[0].c) * nr_vcpus;
+ filesz = sizeof(*ctxt) * nr_vcpus;
sts = xc_core_shdr_set(shdr, strtab, XEN_DUMPCORE_SEC_PRSTATUS,
SHT_PROGBITS, offset, filesz,
__alignof__(ctxt[0].c), sizeof(ctxt[0].c));
@@ -755,7 +755,7 @@
goto out;
/* prstatus: .xen_prstatus */
- sts = dump_rtn(args, (char *)&ctxt[0].c, sizeof(ctxt[0].c) *
nr_vcpus);
+ sts = dump_rtn(args, (char *)&ctxt[0].c, sizeof(*ctxt) * nr_vcpus);
if ( sts != 0 )
goto out;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2009-Nov-24 08:48 UTC
Re: [Xen-devel] [PATCH] libxc: failure to save/restore 32bit HVM guest
On 23/11/2009 23:14, "Mukesh Rathor" <mukesh.rathor@oracle.com> wrote:> Following appears to be an obvious bug. Save/restore of HVM guests > works fine after the following fix. Including the patch, please let me > know if you agree. I''ll make parallel fix in our previous versions > here.Save/restore of HVM guests should work fine before this fix too, since nothing in xc_core.c is used for guest save/restore. It''s used for core dumps, as the name implies, and which is a rather different thing. Furthermore I think the code in xc_core.c is correct as is. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mukesh Rathor
2009-Nov-24 19:23 UTC
Re: [Xen-devel] [PATCH] libxc: failure to save/restore 32bit HVM guest
On Tue, 24 Nov 2009 08:48:16 +0000 Keir Fraser <keir.fraser@eu.citrix.com> wrote:> On 23/11/2009 23:14, "Mukesh Rathor" <mukesh.rathor@oracle.com> wrote: > > > Following appears to be an obvious bug. Save/restore of HVM guests > > works fine after the following fix. Including the patch, please let > > me know if you agree. I''ll make parallel fix in our previous > > versions here. > > Save/restore of HVM guests should work fine before this fix too, since > nothing in xc_core.c is used for guest save/restore. It''s used for > core dumps, as the name implies, and which is a rather different > thing. > > Furthermore I think the code in xc_core.c is correct as is. > > -- Keir >Sorry, my bad, I meant dump core. I was simultaneously looking at save/restore, and accidentally said that. Anyways, since 32bit HVM returns 64bit contexts, it appears the way it is, is buggy, since the sizeof(ctxt[0].c) will return 32bit context size in 32 bit libxc. Infact, after making the fix, user is able to get proper core of a Java HVM guest. Can you please quickly explain why you think it''s correct in such case? thanks, Mukesh _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Nov-24 19:58 UTC
Re: [Xen-devel] [PATCH] libxc: failure to save/restore 32bit HVM guest
On 24/11/2009 19:23, "Mukesh Rathor" <mukesh.rathor@oracle.com> wrote:> Anyways, since 32bit HVM returns 64bit contexts, it appears the way it > is, is buggy, since the sizeof(ctxt[0].c) will return 32bit context > size in 32 bit libxc. Infact, after making the fix, user is able to get > proper core of a Java HVM guest. Can you please quickly explain why you > think it''s correct in such case?Ah, maybe you are right after all. But shouldn''t you change the last two arguments in the call: sts = xc_core_shdr_set(shdr, strtab, XEN_DUMPCORE_SEC_PRSTATUS, SHT_PROGBITS, offset, filesz, __alignof__(ctxt[0].c), sizeof(ctxt[0].c)); ...to reference *ctxt instead of ctxt[0].c, as well? If you agree, send an updated patch and I''ll apply it. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mukesh Rathor
2009-Nov-25 19:16 UTC
Re: [Xen-devel] [PATCH] libxc: failure to save/restore 32bit HVM guest
On Tue, 24 Nov 2009 19:58:48 +0000 Keir Fraser <keir.fraser@eu.citrix.com> wrote:> On 24/11/2009 19:23, "Mukesh Rathor" <mukesh.rathor@oracle.com> wrote: > > > Anyways, since 32bit HVM returns 64bit contexts, it appears the way > > it is, is buggy, since the sizeof(ctxt[0].c) will return 32bit > > context size in 32 bit libxc. Infact, after making the fix, user is > > able to get proper core of a Java HVM guest. Can you please quickly > > explain why you think it''s correct in such case? > > Ah, maybe you are right after all. But shouldn''t you change the last > two arguments in the call: > sts = xc_core_shdr_set(shdr, strtab, XEN_DUMPCORE_SEC_PRSTATUS, > SHT_PROGBITS, offset, filesz, > __alignof__(ctxt[0].c), sizeof(ctxt[0].c)); > ...to reference *ctxt instead of ctxt[0].c, as well? > > If you agree, send an updated patch and I''ll apply it. > > -- Keir >Yup, attached please find updated patch. I had the user test it out also. Since, ctxt is a union, the penultimate argument should be fine. thanks, mukesh Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> diff -r c8caa281a75a tools/libxc/xc_core.c --- a/tools/libxc/xc_core.c Mon Nov 23 08:06:54 2009 +0000 +++ b/tools/libxc/xc_core.c Wed Nov 25 11:15:20 2009 -0800 @@ -628,10 +628,10 @@ PERROR("Could not get section header for .xen_prstatus"); goto out; } - filesz = sizeof(ctxt[0].c) * nr_vcpus; + filesz = sizeof(*ctxt) * nr_vcpus; sts = xc_core_shdr_set(shdr, strtab, XEN_DUMPCORE_SEC_PRSTATUS, SHT_PROGBITS, offset, filesz, - __alignof__(ctxt[0].c), sizeof(ctxt[0].c)); + __alignof__(ctxt[0].c), sizeof(*ctxt)); if ( sts != 0 ) goto out; offset += filesz; @@ -755,7 +755,7 @@ goto out; /* prstatus: .xen_prstatus */ - sts = dump_rtn(args, (char *)&ctxt[0].c, sizeof(ctxt[0].c) * nr_vcpus); + sts = dump_rtn(args, (char *)&ctxt[0].c, sizeof(*ctxt) * nr_vcpus); if ( sts != 0 ) goto out; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel