Re: [PATCH v1 3/3] i386/sev: Add KVM_EXIT_SNP_REQ_CERTS support for certificate-fetching

[Daniel P . Berrangé]

On Thu, Dec 19, 2024 at 11:49:49AM -0600, Michael Roth wrote:
> On Thu, Dec 19, 2024 at 01:37:18PM +0000, Daniel P. Berrangé wrote:
> > IMHO we msut consider unlink() to be a valid thing, because the right
> > way for apps to perform crash safe atomic updates of existing files,
> > is to use rename() from a temporary file, and the rename() in has an
> > implicit unlink as part of its operation. ie apps would be doing:
> > 
> >    fd = open("foo.tmp")
> >    write(fd, ...)
> >    fsync(fd)
> >    close(fd)
> >    rename("foo.tmp", "foo")
> 
> If we still want to allow for this rather than enforcing in-place
> update, one alternative would be to just allow a separate lock file
> to be specified rather than locking the certificate file itself. That
> would provide a bit more flexibility.
> 
> I can update the QEMU implementation to take -certs-lock-file in
> addition to -certs-file so they can be specified separately. And if
> -certs-lock-file is not specified then QEMU will just assume
> management handles things different or has agreed to not do endorsement
> key updates while SNP guests are running.
> 
> I think we'd considered something like that originally but the thinking
> was that locking the certs themselves was more organic in terms of an
> "obvious"/natural solution. But it does end up being a bit more
> inflexible WRT how libraries/etc. might manage file updates underneath
> the covers, so maybe a lock file is the better approach after all.

If we want locking, I think locking the certs file directly is a nicer
idea, as it avoids everyone having to agree on the location of the
lock file, relative to the certs file.

The current locking code just needs to go inside a while(1) loop and
have fstat + stat added to detect the recreation race. For example:

  https://gitlab.com/libvirt/libvirt/-/blob/master/src/util/virpidfile.c#L376

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|