[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] conf: Support for Block Device IO Limits



On 08/29/2012 08:48 AM, Viktor Mihajlovski wrote:
> Introducing a new iolimits element allowing to override certain
> properties of a guest block device like the physical and logical
> block size.
> This can be useful for platforms with 'non-standard' disk formats
> like S390 DASD with its 4K block size.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov linux vnet ibm com>
> ---
>  docs/formatdomain.html.in     |   18 +++++++++++++++
>  docs/schemas/domaincommon.rng |   17 ++++++++++++++
>  src/conf/domain_conf.c        |   49 +++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h        |    5 ++++
>  4 files changed, 89 insertions(+), 0 deletions(-)
> 

> +++ b/src/conf/domain_conf.c
> @@ -3398,6 +3398,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>      char *authUUID = NULL;
>      char *usageType = NULL;
>      char *tray = NULL;
> +    char *logical_block_size = NULL;
> +    char *physical_block_size = NULL;
>  
>      if (VIR_ALLOC(def) < 0) {
>          virReportOOMError();
> @@ -3409,6 +3411,9 @@ virDomainDiskDefParseXML(virCapsPtr caps,
>      def->geometry.sectors = 0;
>      def->geometry.trans = VIR_DOMAIN_DISK_TRANS_DEFAULT;
>  
> +    def->iolimits.logical_block_size = 0;
> +    def->iolimits.physical_block_size = 0;

zero-initialization should be the default, but you are following
existing practice of explicit setting, so it doesn't hurt that much.

> +                physical_block_size =
> +                    virXMLPropString(cur, "physical_block_size");
> +                if (physical_block_size &&
> +                    virStrToLong_ui(physical_block_size, NULL, 0,
> +                                    &def->iolimits.physical_block_size) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("invalid physical block size '%s'"),
> +                                   physical_block_size);

Should we be validating that the result is a power of 2, and at least
512?  But that's just icing to prevent stupidity; it's not a corner case
that I think any sane person would ever try to violate.

ACK once the 0.10.2 tweak is made in the docs.  If patch 2 is fine, I
will make the tweak and push, without needing a v2 from you.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]