From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38595) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8rB3-0004Va-TC for qemu-devel@nongnu.org; Wed, 07 Jan 2015 09:01:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8rAz-0001De-Q6 for qemu-devel@nongnu.org; Wed, 07 Jan 2015 09:01:37 -0500 Received: from mail-we0-x232.google.com ([2a00:1450:400c:c03::232]:53735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8rAz-0001DH-KU for qemu-devel@nongnu.org; Wed, 07 Jan 2015 09:01:33 -0500 Received: by mail-we0-f178.google.com with SMTP id p10so1209563wes.23 for ; Wed, 07 Jan 2015 06:01:33 -0800 (PST) Date: Wed, 7 Jan 2015 12:55:09 +0000 From: Stefan Hajnoczi Message-ID: <20150107125509.GC18014@stefanha-thinkpad.redhat.com> References: <1420511101-8352-1-git-send-email-sfeldma@gmail.com> <1420511101-8352-9-git-send-email-sfeldma@gmail.com> <20150106151220.GO29775@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="kVXhAStRUZ/+rrGn" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2 07/10] rocker: add new rocker switch device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Scott Feldman Cc: john fastabend , Roopa Prabhu , =?utf-8?B?SmnFmcOtIFDDrXJrbw==?= , qemu-devel@nongnu.org --kVXhAStRUZ/+rrGn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 06, 2015 at 08:45:44AM -0800, Scott Feldman wrote: > On Tue, Jan 6, 2015 at 7:12 AM, Stefan Hajnoczi wrot= e: > > On Mon, Jan 05, 2015 at 06:24:58PM -0800, sfeldma@gmail.com wrote: > >> From: Scott Feldman > >> > >> Rocker is a simulated ethernet switch device. The device supports up = to 62 > >> front-panel ports and supports L2 switching and L3 routing functions, = as well > >> as L2/L3/L4 ACLs. The device presents a single PCI device for each sw= itch, > >> with a memory-mapped register space for device driver access. > >> > >> Rocker device is invoked with -device, for example a 4-port switch: > >> > >> -device rocker,name=3Dsw1,len-ports=3D4,ports[0]=3Ddev0,ports[1]=3Dd= ev1, \ > >> ports[2]=3Ddev2,ports[3]=3Ddev3 > >> > >> Each port is a netdev and can be paired with using -netdev id=3D. > > > > This design looks good, it fits the QEMU network subsystem. > > > > Please follow QEMU coding style, for example, using typedefs for structs > > instead of "struct tag". Details are in ./HACKING, ./CODING_STYLE, and > > you can scan patches with QEMU's scripts/checkpatch.pl. >=20 > The patches are already scripts/checkpatch.pl clean. >=20 > And we did follow the HACKING and CODING_STYLE guidelines, with the > exception of typedefs for structs. Did you spot anything else > out-of-compliance? No, just the lack of typedef struct caught my eye. > On typedefs for structs, there are plenty of examples in Qemu of not > following that rule. Perhaps this rule can be enforced by > checkpatch.pl? Personally, I feel that typedef on a struct makes the > code harder to read as it obfuscate the type. For example, typedef > union {...} foo or typedef struct {...} foo or typedef enum {...} foo: > there is no way to tell with foo bar what bar is, at a glance, whereas > union foo bar or struct foo bar or enum foo bar is clear. It seems checkpatch.pl doesn't enforce the rule. There is old code that doesn't follow the coding standard, but new code should. > We can make the typedef change for v3 if it's a hard requirement for incl= usion. Thank you! --kVXhAStRUZ/+rrGn Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUrSytAAoJEJykq7OBq3PILhAH/3JEmj7Q+X4lpmPrARsETECl rq4aksGO4mRPgrgYxG2Z3KgNivOSvTuEmuCVcmPtd8kjEOzJvfn43Y9K9dos+S0a iyhv+xM5iJVlKc3qusZNtS6e0nDgI8moiK8ZISdzhndRmBNDqmnzTls3z/ejcjYx hRhMpoFAjxwYsdodxHZ2FLw86iHnZau9Fme1eohFPIWhb08BLHuYWWgrQQxOd/N/ iMJfKfbJzFH1mdoSDMZG8SrHV6v6zldHjOR2IDbt8lwMdjwy7p9nfTyt10mM//T3 gKJWL8Z4ib/cO/L140Y9KzDuerEANJwecb50Uxn22SRHBr31SEGXDd9RtIp0alU= =DL+H -----END PGP SIGNATURE----- --kVXhAStRUZ/+rrGn--