Attached is the qemu side change for battery support within HVM guest. This will be followed by a xen power management daemon patch. Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com> Thanks, Kamala _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan writes ("[xen-devel][PATCH] Battery Management qemu patch"):> Attached is the qemu side change for battery support within HVM guest. > This will be followed by a xen power management daemon patch.You have added some functions to xenstore.c, but these functions do not encapsulate the xenstore interaction and structure for the power management setup. But they''re just plain xenstore accessors which seem a bit pointless really. Instead you should move the knowledge about the xenstore structure, to xenstore.c, and provide a structured interface between the two components. This will involve moving much of the contents of get_battery_data_from_xenstore and also the relevant part of battery_port_1_writeb. The point is to ensure that nothing outside xenstore.c needs to know about the structure and semantics of xenstore (including the paths, the value formats, and the temporal/causal relationships between the keys and values). This will make it easier when these changes have to go upstream. Why have you selected CAPITAL LETTERS for your new xenstore path ? Very little else in xenstore uses caps and I think it would probably be better avoided here. The patch contains a diff to the file `i386-dm/Makefile'' but that is a symlink to Makefile.target created by the configuration step (invoked in our builds as xen-xetup). Makefile.target has the same diff. This makes the patch slightly troublesome to apply. Also, if you need to add object files please add them in xen-hooks.mak, not directly in Makefile.target. That way I won''t have to deal with merge conflicts. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kamala Narasimhan
2008-Oct-21 19:14 UTC
RE: [xen-devel][PATCH] Battery Management qemu patch
I have attached with this email a revised patch that should address all the issues mentioned below. Please let me know if you have further suggestions. Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com> Thanks, Kamala> -----Original Message----- > From: Ian Jackson > Sent: Tuesday, October 21, 2008 5:27 AM > To: Kamala Narasimhan > Cc: xen-devel@lists.xensource.com > Subject: Re: [xen-devel][PATCH] Battery Management qemu patch > > Kamala Narasimhan writes ("[xen-devel][PATCH] Battery Management qemu > patch"): > > Attached is the qemu side change for battery support within HVMguest.> > This will be followed by a xen power management daemon patch. > > You have added some functions to xenstore.c, but these functions do > not encapsulate the xenstore interaction and structure for the power > management setup. But they''re just plain xenstore accessors which > seem a bit pointless really. Instead you should move the knowledge > about the xenstore structure, to xenstore.c, and provide a structured > interface between the two components. > > This will involve moving much of the contents of > get_battery_data_from_xenstore and also the relevant part of > battery_port_1_writeb. The point is to ensure that nothing outside > xenstore.c needs to know about the structure and semantics of xenstore > (including the paths, the value formats, and the temporal/causal > relationships between the keys and values). This will make it easier > when these changes have to go upstream. > > Why have you selected CAPITAL LETTERS for your new xenstore path ? > Very little else in xenstore uses caps and I think it would probably > be better avoided here. > > > The patch contains a diff to the file `i386-dm/Makefile'' but that is a > symlink to Makefile.target created by the configuration step (invoked > in our builds as xen-xetup). Makefile.target has the same diff. This > makes the patch slightly troublesome to apply. > > > Also, if you need to add object files please add them in > xen-hooks.mak, not directly in Makefile.target. That way I won''t have > to deal with merge conflicts. > > > Ian._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel