[libvirt] [RFC PATCH] threshold: new API virDomainBlockSetWriteThreshold

Tue May 26 14:22:23 UTC 2015

On 05/26/2015 04:25 AM, Daniel P. Berrange wrote:
> On Mon, May 25, 2015 at 11:11:14AM +0200, Peter Krempa wrote:
>> On Sat, May 23, 2015 at 21:45:56 -0600, Eric Blake wrote:
>>> qemu 2.3 added a new QMP command block-set-write-threshold,
>>> which allows callers to get an interrupt when a file hits a
>>> write threshold, rather than the current approach of repeatedly
>>> polling for file allocation.  This patch prepares the API for
>>> callers to register to receive the event, as well as a way
>>> to query the threshold.
>>>

>>>
>>> Note that the threshold is registered as a double, but reported
>>> as an unsigned long long.  This is intentional, as it allows
>>> the use of a flag for registering a threshold via a percentage,
>>> but once registered, the threshold is normalized according to
>>> the current size of the disk.
>>>

>>>  /**
>>> + * VIR_DOMAIN_BLOCK_STATS_WRITE_THRESHOLD:
>>> + *
>>> + * Macro represents the typed parameter, as an llong, that reports
>>
>> Signed? The rest of the block stats fields is signed for a reason, is
>> there a reason why it's here?
>>
>>> + * the threshold in bytes at which the block device will trigger a
>>> + * VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD, or 0 if no threshold is registered.
>>
>> In case it remains signed, the negative values should be somehow
>> described.

Yeah, this one should be ullong.  Will fix.

>>>                                                    unsigned int flags);
>>> +
>>> +typedef enum {
>>> +    /* threshold is a percentage rather than byte limit */
>>> +    VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE = (1 << 0),
>>> +} virDomainBlockSetWriteThresholdFlags;
>>> +int virDomainBlockSetWriteThreshold(virDomainPtr dom,
>>> +                                    const char *disk,
>>> +                                    double threshold,
>>
>> Double? Your last proposal used unsigned long long. Using double with
>> byte sizes seems a bit odd to me. The file size won't exceed unsigned
>> long long for a while so I don't see a reason to use double here.
> 
> Yes, I'm inclined to say u long long too, since that's what we use throughout
> the APIs for representing disk image sizes. So in the unlikely event that
> we do hit the limits of unsigned long long, we'd have countless APIs to
> change regardless.

I intended that:

virDomainSetWriteThreshold(dom, disk, 10*1024*1024*1024, 0)

would be the obvious way of setting a threshold at 10G, and that:

virDomainSetWriteThreshold(dom, disk, 81.47295,
VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE)

would set the disk to 81.47295% of the current size (rounded to a
suitable granularity, even if such rounding is merely truncating to the
nearest int).

Basically, the code I added in libvirt-domain.c enforces that value >= 0
(negative and NaN are not permitted; -0.0 behaves the same as +0.0), and
that value <= ULLONG_MAX (for flags == 0) or <= 100.0 (for flags
specifying percentage).  When flags is 0, this means that for values <
2^53, the byte value is accurate if the user computes their limit in
unsigned long long and uses automatic conversion to double; for values
between 2^53 and 2^64, you lose precision on the low end [but who needs
that much granularity on disks that big, right?], and values > 2^64
(including +inf) are forbidden - so we are still handling things in
unsigned long long.  If someone plays games by passing 12345678.9, the
code will round to a nearby int (whether 12345678 or 12345679 probably
won't matter too much, but the docs tried to make it clear that it is up
to the hypervisor what rounding occurs).  And no matter what the user
passes in, the number is converted to unsigned long long before use, and
all output functions report it in ullong.

The main reason I wanted double was to make percentages more natural; by
using a double, the user is in full control of how much precision to use
(100.1, 100.1234564, ...), and for disks smaller than 2^53, the user can
use percentage to get to a specific byte value.  Whereas with fixed
point, we are limiting users to a fixed amount of precision (if we take
solely 0-100, the user can't do any less than 1% of disk size; or if we
state the range is 0-100000, where 80123 is 80.123%, then the user still
can't get any finer than a megabyte on a multi-gigabyte image).  Again,
querying the threshold will show the user what the actual value in bytes
was computed to be (again allowing for the hypervisor to round to an
appropriate granularity if necessary).

But I can convert my code back to taking threshold as ullong, and
providing the flag as fixed point rather than floating point for
requesting percentages, if that is desired.  If so, what fixed point
should I pick, a thousandth of a percentage (0-100000)?

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150526/95a8ff77/attachment-0001.sig>