Re: [libvirt] ZFS backend does fail if used on non top level pools
> This one is the "unknown" for me. What happens if you create
> Xzfs/images/vol1 (or your command below) without first creating
Xzfs/images?
Answer: it fails, unless you give the '-p' flag.
-p
Creates all the non-existing parent datasets. Datasets
created in this manner
are automatically mounted according to the mountpoint
property inherited from
their parent. Any property specified on the command line
using the -o option
is ignored. If the target filesystem already exists,
the operation completes
successfully.
Example: given an existing zfs pool called "zfs":
# zfs create zfs/foo/bar
cannot create 'zfs/foo/bar': parent does not exist
# zfs create -p zfs/foo/bar
# zfs list zfs/foo
NAME USED AVAIL REFER MOUNTPOINT
zfs/foo 192K 23.5G 96K /zfs/foo
# zfs list -r zfs/foo
NAME USED AVAIL REFER MOUNTPOINT
zfs/foo 192K 23.5G 96K /zfs/foo
zfs/foo/bar 96K 23.5G 96K /zfs/foo/bar
However, I don't see this as a problem for libvirt. The parent should
already exist when you define the pool, and I expect libvirt will only
create immediate children.
If one digs into the virStorageBackendZFSBuildPool they will see libvirt
pool create/build processing would "zpool create $name $path[0...n]"
where $name is the "source.name" (in your case Xzfs/images) and
$path[0...n] would be the various paths (in your case tmp/Xzfs)
Just to be clear, creating a zpool ("zpool create") is different to
creating a zfs dataset ("zfs create").
By analogy to LVM: a zpool is like a volume group, and a zfs
dataset/zvol is like a logical volume.
A zpool (or VG) is created from a collection of block devices - or
something which looks like a block device, e.g. a partition or a
loopback-mounted file. Those are $path[0...n] in the above, and would
be called "physical volumes" in LVM.
"zfs create" then creates a dataset (filesystem) or zvol (block device)
which draws space out of the zpool. The analogous operation in LVM is
"lvcreate", although it will only give you a block device - it's up to
you to make a filesystem within it.
In summary:
zpool create ==> vgcreate (*)
zfs create -V ==> lvcreate
(*) LVM also requires you to label the block devices with "pvcreate"
before you can add them to a volume group. zpool create doesn't require
this.
From my point of view, as a libvirt user: I *could* dedicate an entire
zpool to libvirt, but I don't want to. It would mean libvirt has full
ownership of that set of physical disks, and I may want to use the space
for other things as well.
What I want to do is to allow libvirt to use an existing zpool, with a
parent dataset which it can allocate underneath, like this:
zfs create zfs/libvirt
virsh pool-define-as --name zfs --source-name zfs/libvirt --type zfs
(instead of using pool-create/pool-build). This not only makes it clear
which datasets belong to libvirt, but allows me to do things like
storage accounting at the parent dataset level.
And actually, this almost works. It's just the pool refresh which fails,
because it tries to treat "zfs/images" as if it were a zpool. Stripping
off everything up to the first slash for "zpool get" would fix this.
Arguably this uncovers a couple of other related issues to do with error
handling:
- to the end user, "virsh pool-refresh" silently appears to work (unless
you dig down deep into logs), even though the underlying "zpool get"
returns with an error
- by this stage, pool-refresh has already destroyed all existing libvirt
volumes which were previously in the pool
Regards,
Brian.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: better error checking for LOCAL_PEERCRED
On 17/10/2013 13:29, Eric Blake wrote: On the other hand, making someone chase a URL is less convenient than attaching the patch (ideally, sending inline the way 'git send-email' does things is preferred, but since 'git am' can also handle attachments, it's still easier on the maintainer to send through the list). That said, I went ahead and did the work for you this time around. Thank you Eric. Sorry for making you jump through hoops! Regards, Brian. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: better error checking for LOCAL_PEERCRED
On 15/10/2013 12:16, Daniel P. Berrange wrote: Unfortunately your patch does not apply since your mail client has messed up line wrapping. Also there have been conflicting changes to the code since your patch. I would fix it myself, but I don't have ability to compile test code on BSD platforms. Can you update your patch & re-send. Sorry about that. Updated patch uploaded to https://bugzilla.redhat.com/show_bug.cgi?id=1019929 where Thunderbird can't mangle it. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: better error checking for LOCAL_PEERCRED
I was debugging libvirt with OSX today, and got as far as finding the
problem with LOCAL_PEERCRED, then googled this only to find that Ryota
Ozaki had fixed the problems a few days ago!
However you still may find the following patch useful. It tightens up
the checking in the LOCAL_PEERCRED block, and in particular fixes the
unlocking of the socket in the error return path for invalid groups, by
using the same logic from SO_PEERCRED - have a 'goto cleanup' in all
return paths.
(Detail: I found that when getsockopt was being called with SOL_SOCKET,
cr_ngroups was typically <0, probably because it was uninitialised.
However once the check for this was tightened, it hung because the
socket wasn't being unlocked on return. So better to (a) initialise it
to a negative value anyway, and (b) fix the return path)
However I have not checked that NGROUPS is defined on other BSD-like
systems.
Regards,
Brian Candler.
--- src/rpc/virnetsocket.c.orig 2013-10-10 22:37:49.0 +0100
+++ src/rpc/virnetsocket.c 2013-10-12 22:51:57.0 +0100
@@ -1157,8 +1157,10 @@
{
struct xucred cr;
socklen_t cr_len = sizeof(cr);
+int ret = -1;
virObjectLock(sock);
+cr.cr_ngroups = -1;
# if defined(__APPLE__)
if (getsockopt(sock->fd, SOL_LOCAL, LOCAL_PEERCRED, &cr, &cr_len)
< 0) {
# else
@@ -1166,20 +1168,19 @@
# endif
virReportSystemError(errno, "%s",
_("Failed to get client socket identity"));
-virObjectUnlock(sock);
-return -1;
+goto cleanup;
}
if (cr.cr_version != XUCRED_VERSION) {
virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Failed to get valid client socket identity"));
-return -1;
+goto cleanup;
}
-if (cr.cr_ngroups == 0) {
+if (cr.cr_ngroups <= 0 || cr.cr_ngroups > NGROUPS) {
virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Failed to get valid client socket identity
groups"));
-return -1;
+goto cleanup;
}
/* PID and process creation time are not supported on BSDs */
@@ -1188,8 +1189,11 @@
*uid = cr.cr_uid;
*gid = cr.cr_gid;
+ret = 0;
+
+cleanup:
virObjectUnlock(sock);
-return 0;
+return ret;
}
#else
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: better error checking for LOCAL_PEERCRED
I was debugging libvirt with OSX today, and got as far as finding the
problem with LOCAL_PEERCRED, then googled this only to find that Ryota
Ozaki had fixed the problems a few days ago!
However you still may find the following patch useful. It tightens up
the checking in the LOCAL_PEERCRED block, and in particular fixes the
unlocking of the socket in the error return path for invalid groups, by
using the same logic from SO_PEERCRED - have a 'goto cleanup' in all
return paths.
(Detail: I found that when getsockopt was being called with SOL_SOCKET,
cr_ngroups was typically <0, probably because it was uninitialised.
However once the check for this was tightened, it hung because the
socket wasn't being unlocked on return. So better to (a) initialise it
to a negative value anyway, and (b) fix the return path)
However I have not checked that NGROUPS is defined on other BSD-like
systems. You could just have "if (cr.cr_ngroups <= 0)" instead.
Regards,
Brian Candler.
--- src/rpc/virnetsocket.c.orig2013-10-10 22:37:49.0 +0100
+++ src/rpc/virnetsocket.c2013-10-12 22:51:57.0 +0100
@@ -1157,8 +1157,10 @@
{
struct xucred cr;
socklen_t cr_len = sizeof(cr);
+int ret = -1;
virObjectLock(sock);
+cr.cr_ngroups = -1;
# if defined(__APPLE__)
if (getsockopt(sock->fd, SOL_LOCAL, LOCAL_PEERCRED, &cr, &cr_len)
< 0) {
# else
@@ -1166,20 +1168,19 @@
# endif
virReportSystemError(errno, "%s",
_("Failed to get client socket identity"));
-virObjectUnlock(sock);
-return -1;
+goto cleanup;
}
if (cr.cr_version != XUCRED_VERSION) {
virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Failed to get valid client socket identity"));
-return -1;
+goto cleanup;
}
-if (cr.cr_ngroups == 0) {
+if (cr.cr_ngroups <= 0 || cr.cr_ngroups > NGROUPS) {
virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Failed to get valid client socket identity
groups"));
-return -1;
+goto cleanup;
}
/* PID and process creation time are not supported on BSDs */
@@ -1188,8 +1189,11 @@
*uid = cr.cr_uid;
*gid = cr.cr_gid;
+ret = 0;
+
+cleanup:
virObjectUnlock(sock);
-return 0;
+return ret;
}
#else
int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
