Re: [libvirt] [PATCH 1/1] Refactor HAL Linux code

On Fri, Jun 19, 2009 at 10:07:36AM +0200, Daniel Veillard wrote: > On Thu, Jun 18, 2009 at 09:40:21PM -0400, David Allan wrote: >> * src/node_device_hal_linux.c, src/node_device.c: Older kernels had >> vport_create and delete in /sys/class/scsi_host not /sys/class/fc_host. >> This patch causes libvirt to look in both places. > Important patch as it allows NPIV support to work both in Fedora and > older RHEL kernels. > >> diff --git a/src/node_device.c b/src/node_device.c >> index d01695d..4a936de 100644 >> --- a/src/node_device.c >> +++ b/src/node_device.c >> --- a/src/node_device.h >> +++ b/src/node_device.h >> @@ -30,7 +30,7 @@ > Okay that part of the patch looks just fine. > >> diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c >> index b76235d..b669a3a 100644 >> --- a/src/node_device_hal_linux.c >> +++ b/src/node_device_hal_linux.c >> @@ -34,58 +34,82 @@ >> >> #ifdef __linux__ >> >> -int check_fc_host_linux(union _virNodeDevCapData *d) >> + >> +static int fc_file_exists(const char *prefix, >> + int host, >> + const char *file) > But diff makes an horrible mess here ! > > After looking at the file changes, I get that get_wwn() disapears, > it's inlined in check_fc_host_linux() twice and diff gets lost :-) > I'm not 100% sure why node_device_hal_linux.c needs such a treatment > since the test for both paths actually occurs in src/node_device.c > nodeDeviceVportCreateDelete() but the resulting code is actually smaller, I had pretty much the same thought. The first part of the patch looks good relating to the initial subject. The huge diff following that doesn't seem related to the moving of vport file in sysfs ?