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 -=| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list