From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jeremy Fitzhardinge <jeremy@goop.org>
Subject: Re: [PATCH v2] pv-ops: register xen pci notifier
Date: Wed, 29 Jul 2009 10:02:35 -0700
Message-ID: <4A7080AB.50105@goop.org>
References: <1248835104-5875-1-git-send-email-weidong.han@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <1248835104-5875-1-git-send-email-weidong.han@intel.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Weidong Han <weidong.han@intel.com>
Cc: xen-devel@lists.xensource.com, Keir.Fraser@eu.citrix.com, jbarnes@virtuousgeek.org
List-Id: xen-devel@lists.xenproject.org

On 07/28/09 19:38, Weidong Han wrote:
> Register the notifier to handle hot-plug devices and SR-IOV devices
> for Xen hypervisor. When a device is hot added or removed, it adds
> or removes it to Xen via hypercalls.
>
> Changes in v2:
> Remove inline #ifdef and the awkward dangling else/#endif construction,
> and rather than using memset, use variable declaration and initializer
> to assign the elements in xen_add_device.
>   

That looks much better.  Just one tiny nit:

> +
> +	if (HANDLE_PCI_IOV && pci_dev->is_virtfn) {
> +		struct physdev_manage_pci_ext manage_pci_ext = {
> +			.bus		= pci_dev->bus->number,
> +			.devfn		= pci_dev->devfn,
> +			.is_extfn	= 0,
> +			.is_virtfn 	= 1,
> +			.physfn.bus	= pci_dev->physfn->bus->number,
> +			.physfn.devfn	= pci_dev->physfn->devfn,
> +		};
> +
> +		r = HYPERVISOR_physdev_op(PHYSDEVOP_manage_pci_add_ext,
> +			&manage_pci_ext);
> +	} else if (pci_ari_enabled(pci_dev->bus) && PCI_SLOT(pci_dev->devfn)) {
> +		struct physdev_manage_pci_ext manage_pci_ext = {
> +			.bus		= pci_dev->bus->number,
> +			.devfn		= pci_dev->devfn,
> +			.is_extfn	= 1,
> +			.is_virtfn 	= 0,
> +			.physfn.bus	= 0,
> +			.physfn.devfn	= 0,
>   

It isn't necessarily to explicitly initialize elements to 0; that will
happen implicitly as a result of using an initializer.

Thanks,
    J