On Wed, Oct 09, 2013 at 11:40:35AM +0200, Ján Tomko wrote:
> On 10/08/2013 07:43 PM, Eric Blake wrote:
> > On 10/08/2013 11:28 AM, Ján Tomko wrote:
> >> Commit 76b644c added support for adding RAM filesystems to LXC
> >> domains, with a syntax of:
> >> <source usage='10' unit='MiB'/>
> >> where default unit would be KiB per documentation, however the
> >> XML parser treated sizes without units as bytes.
> >>
> >> When formatting the XML, this was divided by 1024, but the KiB units
> >> were put inside the 'units' attribute, as opposed to the 'unit'
> >> attribute the parser looks for.
> >>
> >> The code generating the mount options assumed the size in the domain
> >> definition to be in KiB, despite it being parsed as B. This worked
> >> as long as exaclty one re-format of the XML happened (for domains that
> >> were just created).
> >>
> >> Change the XML output to bytes and fix the documentation.
> >
> > We still have libvirt in the wild that outputs bogus units= in the XML;
> > that should still pass the RelaxNG grammar even if it is otherwise ignored.
> >
> >> <code>name</code> attribute must be used with
> >> <code>type='template'</code>, and the <code>dir</code> attribute must
> >> be used with <code>type='mount'</code>. The <code>usage</code> attribute
> >> - is used with <code>type='ram'</code> to set the memory limit in KB.
> >> + is used with <code>type='ram'</code> to set the memory limit in bytes.
> >
> > If the RelaxNG is fixed to parse the broken output, then this could
> > document that <code>units</code> is ignored.
> >
>
> >> +++ b/src/conf/domain_conf.c
> >> @@ -14764,8 +14764,8 @@ virDomainFSDefFormat(virBufferPtr buf,
> >> break;
> >>
> >> case VIR_DOMAIN_FS_TYPE_RAM:
> >> - virBufferAsprintf(buf, " <source usage='%lld' units='KiB'/>\n",
> >> - def->usage / 1024);
> >> + virBufferAsprintf(buf, " <source usage='%lld' unit='B'/>\n",
> >> + def->usage);
> >
> > Wait. This says older libvirt was outputting k, not bytes. If that's
> > true, then we MUST continue to output k.
> >
>
> I agree. Even though the number was output in to MiB, GiB, ... after the
> definition was copied, but the output was in KiB for domains that had a chance
> of working correclty (transient and freshly-defined persistent domains seem to
> be).
>
> >> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> >> index b1f429c..7c722cc 100644
> >> --- a/src/lxc/lxc_container.c
> >> +++ b/src/lxc/lxc_container.c
> >> @@ -1428,7 +1428,7 @@ static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs,
> >> VIR_DEBUG("usage=%lld sec=%s", fs->usage, sec_mount_options);
> >>
> >> if (virAsprintf(&data,
> >> - "size=%lldk%s", fs->usage, sec_mount_options) < 0)
> >> + "size=%lld%s", fs->usage, sec_mount_options) < 0)
> >
> > I'm also not convince on this change - if the XML contains k by default,
> > then we must continue to pass k to the mount command.
> >
>
> This does not depend on the XML default, but on the internal unit.
>
> Libvirtd outputs the size in KiB, but since 'units' is ignored by the parser,
> libvirt_lxc just puts the raw number in 'usage', making KiB its internal unit.
>
> If we fix the formatter and/or parser to correctly read the value it just
> printed, we will need to either get rid of this 'k' or store it in KiB internally.
This code was added for the benefit of libvirt-sandbox and that is the
main (possibly only) user of this feature currently.
It passes in XML that says
<filesystem type="ram" accessmode="passthrough">
<source usage="10485760" units="bytes"/>
<target dir="/run"/>
</filesystem>
and this translates to a sandbox with a 10MB mount
# cat /proc/mounts | grep /run
tmpfs /run tmpfs rw,context=system_u:object_r:svirt_sandbox_file_t:s0,nosuid,nodev,relatime,size=10240k 0 0
And this is reflected in the output XML
<filesystem type='ram' accessmode='passthrough'>
<source usage='10240' units='KiB'/>
<target dir='/run'/>
</filesystem>
So IMHO we need to fix the parser to honour the 'units' attribute.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list