On Mon, Oct 29, 2007 at 12:03:34PM +0000, Richard W.M. Jones wrote:
General comments. (I haven't yet read the patches line by line,
but
there are some things that I noticed that we can get back to later.)
I patched up my libvirt with all the patches (the remote patches turned
out to be necessary after all for reasons I now understand -- see below).
Out of the box, I can do:
# virsh pool-list
Name State Autostart
-----------------------------------------
Now I have to add my storage pools manually. Even after reading
previous mail on the XML formats, it was very unclear what the correct
format to use was:
# cat /tmp/pool.xml
<pool type="fs">
<name>xenimages</name>
<uuid>12345678-1234-1234-1234-123456781234</uuid>
<target dir="/var/lib/xen/images"/>
<permissions>
<mode>0700</mode>
<owner>0</owner>
<group>0</group>
<label>xen_image_t</label>
</permissions>
</pool>
# src/virsh pool-create /tmp/pool.xml
Pool xenimages created from /tmp/pool.xml
As we did with virsh attach-disk/attach-interface, vs attach-device
I think it would be helpful to provide a 'virsh vol-create' variant
which took a handful of explicit args as an alternative to the XML,
eg virsh pool-create --type fs xenimages /var/lib/xen/images
It wouldn't have the full capabilities to set every single XML
attribute/element, but would be good enough so that admins didn't
need to write the XML for the 95% common case.
The first problem here is that we've got the "secret config files"
again. I know that Xen made this idiotic mistake of hiding the config
files from the world, and in libvirt we have to work around it for
domains, but there's no particular reason why we have to copy this bad,
non-Unix-like design decision into other parts of our API.
The config file are not hidden/secret - they're in the usual libvirt
daemon config location /etc/libvirt/storage/[pool name].xml
As we do with the 'default' network, we can easily ship a canned config
file for say, /var/lib/virt/images as a local directory based pool.
Any other type of pool is going to be site-specific, but we can provide
a selection of common example configs as needed.
I was envisaging a much more straightforward config file:
/etc/libvirt.conf -------------------
disk_storage_pools: [ "/var/lib/xen/images" ]
lvm_volgroups: [ "/dev/MyXenImages" ]
-------------------------------------
That is not sufficient to describe all the metadata for the different
types of storage pool.
With this, you don't need to put storage logic into libvirtd. It
can
all be discovered on demand, just using the config file and the commands
already included in storage_backend_*.c The upshot is that we don't
need a daemon to manage storage pools, except in the remote case where
it's just there to do things remotely.
To be honest even in the local case we need the daemon. The only reason
we previously get away without having a daemon when running locally, is
that the entire virt-manager app has run as root. Long term I think
pretty much all libvirt clients will be going via the daemon, whether
local or remote.
(To be fair, the proposed patch seems to have a config file in
/etc/libvirt/storage/storage.conf, but it's not implemented at the
moment and it is unclear what will go in here, and whether a suitable
default can be created to allow storage pools to default to something
sensible on Fedora).
# src/virsh pool-list
Name State Autostart
-----------------------------------------
libvir: Storage error : pool has no config file
xenimages active no autostart
Opps, that error message is a bug.
There's no pool-info binding in virsh yet, but there is a
corresponding
call at the libvirt level and thankfully the output is a struct.
Yep, I was planning to add a pool-info API to list the capacity vs
allocation of the pool & name, uuid, etc, as with dom-info. Likeiwise
for the vol-info API
Create a volume:
# cat /tmp/vol.xml
<volume>
<name>tempvol</name>
<uuid>12345678-1234-1234-1234-123456781234</uuid>
<capacity>100000</capacity>
<allocation>100000</allocation>
<format>
<type>raw</type>
</format>
<permissions>
<mode>0700</mode>
<owner>0</owner>
<group>0</group>
<label>xen_image_t</label>
</permissions>
</volume>
# src/virsh vol-create xenimages /tmp/vol.xml
(libvirtd dumped core at this point)
The format type was not quite right <format type='raw'>. It obviously
shouldn't dump core though!
NB, <format> is actually optional - it defaults to raw. Permissions
should be optional, but currently aren't. Allocation is optional too,
without it, you will get a sparse file.
And I still don't think that passing XML descriptions is a good
idea.
But because you're envisaging being able to create complex volumes like
qcow, encrypted, etc., I will temper this by saying that we should have
a small core XML which all drivers MUST support.
For example, every driver must support pools and volumes described like
this (verbatim, with no extra fields):
<pool type="xxx">
<name>xenimages</name>
<!-- target should make sense for the xxx driver, eg. path for fs,
LUN name for iSCSI, etc. There is no need for the dir=...
attribute -->
<target>/var/lib/xen/images</target>
<!-- permissions should default to sensible values -->
</pool>
Yep, I intended permissions to be optional. Not all pools have a target,
some only have a source. Basically name,and either target or source
is the minimal set. Everything else should be optional.
and:
<volume>
<name>tempvol</name>
<!-- I noticed that units are missing from original -->
<capacity unit="GiB">10</capacity>
<!-- permissions and format should default to sensible -->
</volume>
Yes, adding units is a good idea. That looks good as a minimal dataset.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules:
http://search.cpan.org/~danberr/ -=|
|=- Projects:
http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|