quoted

quoted

On 31.01.12 at 12:09, Wei Liu [off-list ref] wrote:

On Tue, 2012-01-31 at 09:01 +0000, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 30.01.12 at 18:10, Wei Liu [off-list ref] wrote:

On Mon, 2012-01-30 at 16:35 +0000, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 30.01.12 at 15:45, Wei Liu [off-list ref] wrote:

-int xenvif_map_frontend_rings(struct xenvif *vif,
-			      grant_ref_t tx_ring_ref,
-			      grant_ref_t rx_ring_ref)
+int xenvif_map_frontend_rings(struct xen_comms *comms,
+			      int domid,
+			      unsigned long ring_ref[],
+			      unsigned int  ring_ref_count)
 {
-	void *addr;
-	struct xen_netif_tx_sring *txs;
-	struct xen_netif_rx_sring *rxs;
-
-	int err = -ENOMEM;
+	struct gnttab_map_grant_ref op[NETBK_MAX_RING_PAGES];
+	unsigned int i;
+	int err = 0;
 
-	err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(vif),
-				     tx_ring_ref, &addr);

Any reason why you don't just extend this function (in a prerequisite
patch) rather than open coding a common utility function (twice) here,
so that other backends (blkback!) can benefit later as well.

Jan

I'm mainly focusing on netback stuffs, so the code is slightly coupled
with netback -- NETBK_MAX_RING_PAGES.

To extend xenbus_map_ring_valloc and make more generic, it requires
setting a global maximum page number limits on rings, I think it will
require further investigation and code refactor -- which I have no time
to attend to at the moment. :-/

Why? You can simply pass in the number of pages, there's no need
for a global maximum.

I mean the gnttab_map_gran_ref array, it is statically allocated at the
moment. Of course we can make it dynamically allocated, but why bother
taking the risk of allocation failure.

There's so many other allocations, why would you worry about this
one.

But of course you can undo what a recent change did, and then
subsequently someone else will have to clean up again after you.
I'm just asking to follow good programming practices and write
re-usable code where potential for re-use is obvious (after all,
multi-page block interface patches have been floating around for
much longer than yours for the net interface).

Jan