Re: [xen-devel][PATCH] Battery Management qemu patch

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel