On 03/06/2012 08:14 AM, Peter Krempa wrote: > On 03/06/2012 01:34 AM, Eric Blake wrote: >> Make it obvious to 'dumpxml' readers what unit we are using, >> since our default of KiB for memory (1024) differs from >> qemu's default of MiB; while we use bytes for storage. >> >> Tests were updated via: >> >> +++ b/docs/schemas/basictypes.rng >> @@ -140,8 +140,16 @@ >> >> <define name='unit'> >> <data type='string'> >> -<param name='pattern'>[kKmMgGtTpPeE]</param> >> +<param name='pattern'>(bytes)|[kKmMgGtTpPeE]</param> > > Looking at this again. Don't you want to be able to specify the unit as > "KiB" or just only as "k"? Yes, and that gets fixed later in the series (see 6/15 and 10/15). This was the minimum change to get 'make check' to pass at this stage of the series before I start adding new parsing abilities in later patches. Also, note that domaincommon.rng _isn't_ using <ref name='unit'/> until patch 10/15; in this patch, domaincommon.rng is using an open-coded RNG that accepts _only_ <attribute name='unit'><value>KiB</value></attribute>. >> - virBufferAsprintf(buf, "<currentMemory>%lu</currentMemory>\n", >> + virBufferAsprintf(buf, "<memory unit='KiB'>%lu</memory>\n", >> + def->mem.max_balloon); >> + virBufferAsprintf(buf, "<currentMemory >> unit='KiB'>%lu</currentMemory>\n", >> def->mem.cur_balloon); > > I'm not quite sure about this. When we print the unit and somebody tries > to use the xml as a template for a new machine or simply to change the > memory allocation for the domain using virsh edit, he might be tempted > to change the unit to mibibytes and expect the amount of memory to be > used. Instead of that he will get that amount in kilobytes and no > warning about this. We probably should parse the unit and at least warn > the user about this happening. Yes, that happens in 10/15. > > With this patch applied to current upstream you will need to: > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml > b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml > index 65cd55d..b6bf1d4 100644 > --- a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml > +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml > @@ -1,8 +1,8 @@ > <domain type='qemu'> > <name>QEMUGuest1</name> > <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > - <memory>219136</memory> > - <currentMemory>219136</currentMemory> > + <memory unit='KiB'>219136</memory> > + <currentMemory unit='KiB'>219136</currentMemory> Ah, new tests added in the meantime. Yes, when I rebase, I'll make sure things still work. > > to pass the tests. > > I'm not comfortable ACKing this one :(. I'd like to have some feedback > from others on printing units without parsing them. Otherwise looks fine > and passes the tests with that patch applied. If anything, I can modify the commit message to mention that a later patch will parse units. -- Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list