Message ID | 1416392276-10408-4-git-send-email-tumanova@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Am 19.11.2014 um 11:17 schrieb Ekaterina Tumanova: [...] > @@ -678,6 +683,64 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) > bs->bl.opt_mem_alignment = s->buf_align; > } > > +static int CheckForDASD(int fd) no camelcase for function names check_for_dasd [...]
Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> writes: > This patch introduces driver methods of defining disk blocksizes > (physical and logical) and hard drive geometry. > The method is only implemented for "host_device". For "raw" devices > driver calls child's method. > The detection will only work for DASD devices. In order to check that > a local CheckForDASD function was introduced, which calls BIODASDINFO2 ioctl > and returns 0 only if it succeeds. > > Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> > --- > block/raw-posix.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > block/raw_bsd.c | 12 ++++++++++ > 2 files changed, 77 insertions(+) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 45f1d79..274ceda 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -56,6 +56,7 @@ > #include <linux/cdrom.h> > #include <linux/fd.h> > #include <linux/fs.h> > +#include <linux/hdreg.h> > #ifndef FS_NOCOW_FL > #define FS_NOCOW_FL 0x00800000 /* Do not cow file */ > #endif > @@ -93,6 +94,10 @@ > #include <xfs/xfs.h> > #endif > > +#ifdef __s390__ > +#include <asm/dasd.h> > +#endif > + > //#define DEBUG_FLOPPY > > //#define DEBUG_BLOCK > @@ -678,6 +683,64 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) > bs->bl.opt_mem_alignment = s->buf_align; > } > > +static int CheckForDASD(int fd) As Christian said, no CamelCase for functions. I guess your function returns either 0 or -1. Can't say for sure without looking up BIODASDINFO2. I'd do a slightly higher level is_dasd() returning bool. Your choice. > +{ > +#ifdef BIODASDINFO2 > + struct dasd_information2_t info = {0}; > + > + return ioctl(fd, BIODASDINFO2, &info); > +#endif > + return -1; > +} > + > +static struct ProbeBlockSize hdev_probe_blocksizes(BlockDriverState *bs) > +{ > + BDRVRawState *s = bs->opaque; > + struct ProbeBlockSize block_sizes = {0}; > + > + block_sizes.rc = CheckForDASD(s->fd); > + /* If DASD, get blocksizes */ > + if (block_sizes.rc == 0) { > + block_sizes.size.log = probe_logical_blocksize(bs, s->fd); > + block_sizes.size.phys = probe_physical_blocksize(bs, s->fd); > + } > + > + return block_sizes; > +} Fails unless DASD. Why? The block size concept applies not just to DASD, and the probe_*_blocksize() functions should just work, shouldn't they? > + > +static struct ProbeGeometry hdev_probe_geometry(BlockDriverState *bs) > +{ > + BDRVRawState *s = bs->opaque; > + struct ProbeGeometry geometry = {0}; > + struct hd_geometry ioctl_geo = {0}; Is this initializer really necessary? Because I like my local variable names short & sweet, I'd call this one geo. Your choice. > + > + geometry.rc = CheckForDASD(s->fd); > + if (geometry.rc != 0) { Works if your function really returns either 0 or -1. If it can return a positive value, it breaks the callback's contract. > + goto done; > + } > + /* If DASD, get it's geometry */ its > + geometry.rc = ioctl(s->fd, HDIO_GETGEO, &ioctl_geo); > + /* Do not return a geometry for partition */ > + if (ioctl_geo.start != 0) { > + geometry.rc = -1; > + goto done; > + } > + /* HDIO_GETGEO may return success even though geo contains zeros > + (e.g. certain multipath setups) */ > + if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) { > + geometry.rc = -1; > + goto done; > + } > + if (geometry.rc == 0) { > + geometry.geo.heads = (uint32_t)ioctl_geo.heads; > + geometry.geo.sectors = (uint32_t)ioctl_geo.sectors; > + geometry.geo.cylinders = (uint32_t)ioctl_geo.cylinders; The LHS is uint32_t, the RHS is unsigned char or unsigned short, thus the type casts are superfluous clutter. 8-bit head * 8-bit sectors * 16-bit cylinders can't represent today's disk sizes, so ioctl_geo.cylinders is generally useless. The common advice is to ignore it, and do something like geometry.geo.cylinders = nb_sectors / (ioctl_geo.heads * ioctl_geo.sectors) A possible alternative is to explicitly document that the returned cylinders value can't be trusted. Another one is not to return the number of cylinders :) > + geometry.geo.start = (uint32_t)ioctl_geo.start; Always assigns zero (we checked above). Why is member geo_start useful? > + } > +done: > + return geometry; > +} Also fails unless DASD. The geometry concept applies pretty much only to disks older than thirty years or so, and to software designed for them, like HDIO_GETGEO. > + > static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb) > { > int ret; > @@ -2128,6 +2191,8 @@ static BlockDriver bdrv_host_device = { > .bdrv_get_info = raw_get_info, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > + .bdrv_probe_blocksizes = hdev_probe_blocksizes, > + .bdrv_probe_geometry = hdev_probe_geometry, > > .bdrv_detach_aio_context = raw_detach_aio_context, > .bdrv_attach_aio_context = raw_attach_aio_context, > diff --git a/block/raw_bsd.c b/block/raw_bsd.c > index 401b967..d164eba 100644 > --- a/block/raw_bsd.c > +++ b/block/raw_bsd.c > @@ -173,6 +173,16 @@ static int raw_probe(const uint8_t *buf, int buf_size, const char *filename) > return 1; > } > > +static struct ProbeBlockSize raw_probe_blocksizes(BlockDriverState *bs) > +{ > + return bdrv_probe_blocksizes(bs->file); > +} > + > +static struct ProbeGeometry raw_probe_geometry(BlockDriverState *bs) > +{ > + return bdrv_probe_geometry(bs->file); > +} > + > static BlockDriver bdrv_raw = { > .format_name = "raw", > .bdrv_probe = &raw_probe, > @@ -190,6 +200,8 @@ static BlockDriver bdrv_raw = { > .has_variable_length = true, > .bdrv_get_info = &raw_get_info, > .bdrv_refresh_limits = &raw_refresh_limits, > + .bdrv_probe_blocksizes = &raw_probe_blocksizes, > + .bdrv_probe_geometry = &raw_probe_geometry, > .bdrv_is_inserted = &raw_is_inserted, > .bdrv_media_changed = &raw_media_changed, > .bdrv_eject = &raw_eject,
diff --git a/block/raw-posix.c b/block/raw-posix.c index 45f1d79..274ceda 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -56,6 +56,7 @@ #include <linux/cdrom.h> #include <linux/fd.h> #include <linux/fs.h> +#include <linux/hdreg.h> #ifndef FS_NOCOW_FL #define FS_NOCOW_FL 0x00800000 /* Do not cow file */ #endif @@ -93,6 +94,10 @@ #include <xfs/xfs.h> #endif +#ifdef __s390__ +#include <asm/dasd.h> +#endif + //#define DEBUG_FLOPPY //#define DEBUG_BLOCK @@ -678,6 +683,64 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) bs->bl.opt_mem_alignment = s->buf_align; } +static int CheckForDASD(int fd) +{ +#ifdef BIODASDINFO2 + struct dasd_information2_t info = {0}; + + return ioctl(fd, BIODASDINFO2, &info); +#endif + return -1; +} + +static struct ProbeBlockSize hdev_probe_blocksizes(BlockDriverState *bs) +{ + BDRVRawState *s = bs->opaque; + struct ProbeBlockSize block_sizes = {0}; + + block_sizes.rc = CheckForDASD(s->fd); + /* If DASD, get blocksizes */ + if (block_sizes.rc == 0) { + block_sizes.size.log = probe_logical_blocksize(bs, s->fd); + block_sizes.size.phys = probe_physical_blocksize(bs, s->fd); + } + + return block_sizes; +} + +static struct ProbeGeometry hdev_probe_geometry(BlockDriverState *bs) +{ + BDRVRawState *s = bs->opaque; + struct ProbeGeometry geometry = {0}; + struct hd_geometry ioctl_geo = {0}; + + geometry.rc = CheckForDASD(s->fd); + if (geometry.rc != 0) { + goto done; + } + /* If DASD, get it's geometry */ + geometry.rc = ioctl(s->fd, HDIO_GETGEO, &ioctl_geo); + /* Do not return a geometry for partition */ + if (ioctl_geo.start != 0) { + geometry.rc = -1; + goto done; + } + /* HDIO_GETGEO may return success even though geo contains zeros + (e.g. certain multipath setups) */ + if (!ioctl_geo.heads || !ioctl_geo.sectors || !ioctl_geo.cylinders) { + geometry.rc = -1; + goto done; + } + if (geometry.rc == 0) { + geometry.geo.heads = (uint32_t)ioctl_geo.heads; + geometry.geo.sectors = (uint32_t)ioctl_geo.sectors; + geometry.geo.cylinders = (uint32_t)ioctl_geo.cylinders; + geometry.geo.start = (uint32_t)ioctl_geo.start; + } +done: + return geometry; +} + static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb) { int ret; @@ -2128,6 +2191,8 @@ static BlockDriver bdrv_host_device = { .bdrv_get_info = raw_get_info, .bdrv_get_allocated_file_size = raw_get_allocated_file_size, + .bdrv_probe_blocksizes = hdev_probe_blocksizes, + .bdrv_probe_geometry = hdev_probe_geometry, .bdrv_detach_aio_context = raw_detach_aio_context, .bdrv_attach_aio_context = raw_attach_aio_context, diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 401b967..d164eba 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -173,6 +173,16 @@ static int raw_probe(const uint8_t *buf, int buf_size, const char *filename) return 1; } +static struct ProbeBlockSize raw_probe_blocksizes(BlockDriverState *bs) +{ + return bdrv_probe_blocksizes(bs->file); +} + +static struct ProbeGeometry raw_probe_geometry(BlockDriverState *bs) +{ + return bdrv_probe_geometry(bs->file); +} + static BlockDriver bdrv_raw = { .format_name = "raw", .bdrv_probe = &raw_probe, @@ -190,6 +200,8 @@ static BlockDriver bdrv_raw = { .has_variable_length = true, .bdrv_get_info = &raw_get_info, .bdrv_refresh_limits = &raw_refresh_limits, + .bdrv_probe_blocksizes = &raw_probe_blocksizes, + .bdrv_probe_geometry = &raw_probe_geometry, .bdrv_is_inserted = &raw_is_inserted, .bdrv_media_changed = &raw_media_changed, .bdrv_eject = &raw_eject,
This patch introduces driver methods of defining disk blocksizes (physical and logical) and hard drive geometry. The method is only implemented for "host_device". For "raw" devices driver calls child's method. The detection will only work for DASD devices. In order to check that a local CheckForDASD function was introduced, which calls BIODASDINFO2 ioctl and returns 0 only if it succeeds. Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com> --- block/raw-posix.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ block/raw_bsd.c | 12 ++++++++++ 2 files changed, 77 insertions(+)