On Tue, Feb 24, 2015 at 09:15:40AM -0500, Stefan Berger wrote:
On 02/24/2015 09:08 AM, Martin Kletzander wrote:On Mon, Feb 23, 2015 at 06:50:48AM -0500, Stefan Berger wrote:Pass the TPM file descriptor to QEMU via command line. Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional parameters -add-fd set=10,fd=20. This addresses the use case when QEMU is started with non-root privileges and QEMU cannot open /dev/tpm0 for example. Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx> --- src/qemu/qemu_command.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 539c956..12e2e2f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST, "interleave"); /** + * qemuVirCommandGetFDSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU -add-fd command line option + * for the given file descriptor. The file descriptor must previously + * have been 'transferred' in a virCommandPassFD() call. + * This function for example returns "set=10,fd=20". + */ +static char * +qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd) < 0);^ This line doesn't make much sense, I guess you just wanted to do it | without the comparison to zero here ---------------------------------+ Anyway, is there a reason for passing "set=X,fd=Y" instead of just doing fd=Y and being done with it? I must admit I don't know the details of /dev/fdset, so that might've just missed me.Passing fd by itself is not supported with this device. I guess back then when I implemented I knew I could pass the fd via this /dev/fdset and so fd= doesn't need to be implemented on top of that.
OK, I didn't know that and that's a valid reason. One more thing about this patch though. I just noticed that this is not the first version and there were some previous ones before, so I went and skimmed some of the responses and one is still not solved/decided. Why would we number the set according to our array index instead of just the first FD in that set (for example)? The thing I'm worried about is that when we're passing FDs later on, we might do that and those numbers might clash mixing different FDs in the same set. Having said that I also looked at the code and even though we know how to pass FDs to qemu later on with QMP and add-fd, we do that only when probing for that capability; so it's not decided how we're going to name the sets yet, I guess. That also shows when looking for a local "database" of fdsets per each QEMU that's running => there's none; if we want to use that info after some time (virCommand is cleaned, libvirtd restarted, etc.), then there is no way of telling which ones were used for what. The only place where we send FDs is without FD sets and the FD number is reflected in an alias (e.g. vhost35 or something like that). Anyway, that shouldn't be your concern at all, I just think it would be nice to have fd=x,set=x and then assign an alias to the live domain for the tpm device with that x used. Looking forward to v2, Martin
Attachment:
pgp3Xmp2sfXvB.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list