|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 2/3] vmport: Add VMware provided include files.
On 09/03/14 03:51, Jan Beulich wrote: On 02.09.14 at 20:46, <dslutz@xxxxxxxxxxx> wrote:On 09/02/14 03:34, Jan Beulich wrote:On 01.09.14 at 17:33, <dslutz@xxxxxxxxxxx> wrote:These 2 files: backdoor_def.h and guest_msg_def.h come from: http://packages.vmware.com/tools/esx/3.5latest/rhel4/SRPMS/index.html open-vm-tools-kmod-7.4.8-396269.423167.src.rpm open-vm-tools-kmod-7.4.8.tar.gz vmhgfs/backdoor_def.h vmhgfs/guest_msg_def.h and are unchanged.Okay, I can the value in keeping them as is even if they're in conflict with some of our coding style rules. But ...--- /dev/null +++ b/xen/arch/x86/hvm/vmport/backdoor_def.h @@ -0,0 +1,167 @@ +/* ********************************************************** + * Copyright 1998 VMware, Inc. All rights reserved. + * ********************************************************** + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation version 2 and no later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* + * backdoor_def.h -- + * + * This contains backdoor defines that can be included from + * an assembly language file. + */ + + + +#ifndef _BACKDOOR_DEF_H_ +#define _BACKDOOR_DEF_H_... these guards have the (theoretical at this point) risk of clashing (apart from violating the C standard's naming rules) and ...Yes. That is why I had the comment: /* Note: VMPORT_PORT and VMPORT_MAGIC is also defined as BDOOR_PORT * and BDOOR_MAGIC in backdoor_def.h Defined in vmport.h also. */ because I did not want the reset of xen to be including this file.I can't connect your response to my comment. Reading it a again, I agree. s/reset/rest/ helps a little. Hope this is better: I put these files in the new vmware directory in order to prevent other parts of xen from attempting to use these include files. This is a work around for any possible issue with them. In v2 I had defined in a different file the 2 numbers that are used else where. It now looks like in v3 I can fix this by having all uses contained in this new directory. If I have found the right "C standard's naming rule", this file is not alone in violating it:Yes, that's the one. And no, bad examples elsewhere aren't an excuse to introduce more bad code. Fine with me. I was also attempting to understand how it would need to change. +#define INCLUDE_ALLOW_MODULE +#define INCLUDE_ALLOW_USERLEVEL +#define INCLUDE_ALLOW_VMMEXT +#define INCLUDE_ALLOW_VMCORE +#define INCLUDE_ALLOW_VMKERNEL +#include "includeCheck.h"... the patch is obviously incomplete without this header, the name of which is certainly not in line with our way of naming files.Yes. And an empty file of that name is added in the next patch.Question is - why not here (to make the set self consistent)? I see no reason not to. So I have added it here. So I am getting the feeling is that it would be better to have a patch adjusting these include files to better fit in xen, and then move them out of the new xen/arch/x86/hvm/vmware directory.Not sure what makes you think so - as said earlier on, I can see the value of having the file mostly (if not entirely) identical to what is coming from VMware. That way, eventual future updates are going to be easier. All we need to avoid are eventual future naming clashes. Of course, if the headers are so badly written that the majority of their defines has a risk of clashing, then of course it might indeed be preferable to redo them in Xen style.
You are the 2nd person to point out issues with these files. So I guess
I was jumping the gun in saying to change them and move them. In any
case if I was to do the smallest change to make them better that would
be a 2nd patch.
Thinking more about it, it makes more sense to leave them in the vmware
directory.
-Don Slutz
Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |