From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36648) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bKnfh-0008OS-BW for qemu-devel@nongnu.org; Wed, 06 Jul 2016 10:19:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bKnfb-0008DY-Fg for qemu-devel@nongnu.org; Wed, 06 Jul 2016 10:19:25 -0400 References: <1467732272-23368-1-git-send-email-clord@redhat.com> <1467732272-23368-5-git-send-email-clord@redhat.com> From: Max Reitz Message-ID: <9bc2a125-4a05-a145-0eb4-082d733f8640@redhat.com> Date: Wed, 6 Jul 2016 16:19:05 +0200 MIME-Version: 1.0 In-Reply-To: <1467732272-23368-5-git-send-email-clord@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="mID5ONX7Lw788sv4tcqdF5fcv3NDvgr6O" Subject: Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Colin Lord , qemu-devel@nongnu.org Cc: kwolf@redhat.com, qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --mID5ONX7Lw788sv4tcqdF5fcv3NDvgr6O From: Max Reitz To: Colin Lord , qemu-devel@nongnu.org Cc: kwolf@redhat.com, qemu-block@nongnu.org Message-ID: <9bc2a125-4a05-a145-0eb4-082d733f8640@redhat.com> Subject: Re: [PATCH v3 04/32] blockdev: Move bochs probe into separate file References: <1467732272-23368-1-git-send-email-clord@redhat.com> <1467732272-23368-5-git-send-email-clord@redhat.com> In-Reply-To: <1467732272-23368-5-git-send-email-clord@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 05.07.2016 17:24, Colin Lord wrote: > This puts the bochs probe function into its own separate file as part o= f > the process of modularizing block drivers. Having the probe functions > separate from the rest of the driver allows us to probe without having > to potentially unnecessarily load the driver. >=20 > Signed-off-by: Colin Lord > --- > block/Makefile.objs | 1 + > block/bochs.c | 55 ++----------------------------------= -------- > block/probe/bochs.c | 21 +++++++++++++++++ > include/block/driver/bochs.h | 40 ++++++++++++++++++++++++++++++++ > include/block/probe.h | 6 +++++ > 5 files changed, 70 insertions(+), 53 deletions(-) > create mode 100644 block/probe/bochs.c > create mode 100644 include/block/driver/bochs.h > create mode 100644 include/block/probe.h >=20 [...] > diff --git a/block/probe/bochs.c b/block/probe/bochs.c > new file mode 100644 > index 0000000..8adc09f > --- /dev/null > +++ b/block/probe/bochs.c > @@ -0,0 +1,21 @@ > +#include "qemu/osdep.h" > +#include "block/block_int.h" > +#include "block/probe.h" > +#include "block/driver/bochs.h" > + > +int bochs_probe(const uint8_t *buf, int buf_size, const char *filename= ) > +{ > + const struct bochs_header *bochs =3D (const void *)buf; > + > + if (buf_size < HEADER_SIZE) > + return 0; > + > + if (!strcmp(bochs->magic, HEADER_MAGIC) && > + !strcmp(bochs->type, REDOLOG_TYPE) && > + !strcmp(bochs->subtype, GROWING_TYPE) && > + ((le32_to_cpu(bochs->version) =3D=3D HEADER_VERSION) || > + (le32_to_cpu(bochs->version) =3D=3D HEADER_V1))) > + return 100; > + > + return 0; > +} Not sure what to do about the coding style here. Some people prefer code movement to be pure, but I feel bad about doing code movement and then leaving the code wrongly formatted. That is, in my opinion, every patch should pass checkpatch.pl (but I won't object to a patch that doesn't pass checkpatch.pl simply because of pre-existing code). > diff --git a/include/block/driver/bochs.h b/include/block/driver/bochs.= h > new file mode 100644 > index 0000000..cd87256 > --- /dev/null > +++ b/include/block/driver/bochs.h Kevin's point that we maybe should just put this into block/ itself (just like block/qcow2.h) is not a bad one, but I'm fine either way. > @@ -0,0 +1,40 @@ > +#ifndef BOCHS_H > +#define BOCHS_H Maybe BLOCK_BOCHS_H would be better, considering that Bochs is primarily a system emulator and its image format doesn't really have an own name. (Compare block/qcow2.h, which uses BLOCK_QCOW2_H.) Independently of what you decide to do in any of these three places: Reviewed-by: Max Reitz --mID5ONX7Lw788sv4tcqdF5fcv3NDvgr6O Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEvBAEBCAAZBQJXfRNZEhxtcmVpdHpAcmVkaGF0LmNvbQAKCRA7sUIC6DisrW0T B/9N4wWVH5vX6gQM2jLGykM+FDAAftadzOW5CudVDEUxWd0wV2c80eX54O7eacZ5 0pcQtEZ58Fq9ipMyKvSy2gJQOk15MTNrBPMf4IK7iWoPi1MuMadUwEVjutA42MqL o+0MwU9HlTrhMfdXQGYhBFChUkleQDyx0yb7yipTHlxIb+OK4lg2Bk8+22AVSSbY wDrEXLYmUFDSSYn3Fq+ldsVj7mxM1buOAMh3RUPux1YJDFV/fnhywMi4bVhyy1zB oW5gHGrifVZZpmu/pa4I0kAz2su66nOdU5GGc0Xzp7T00pTkZqPKTF26AUSD2brE gDrdwxzhRPlrWRzhJiQrx89e =mjaP -----END PGP SIGNATURE----- --mID5ONX7Lw788sv4tcqdF5fcv3NDvgr6O--