From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "open list:Block layer core" <qemu-block@nongnu.org>,
Fam Zheng <fam@euphon.net>,
"Hajnoczi, Stefan" <stefanha@redhat.com>,
qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [PATCH 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest
Date: Mon, 29 Jul 2024 18:55:15 +0200 [thread overview]
Message-ID: <ZqfJcz6GTsWglrQ5@redhat.com> (raw)
In-Reply-To: <CABgObfbecac_70Pt4DWKPRm96VCOOCjGOCSB3TkkK490FmvPsg@mail.gmail.com>
Am 29.07.2024 um 14:26 hat Paolo Bonzini geschrieben:
> Il lun 29 lug 2024, 14:20 Kevin Wolf <kwolf@redhat.com> ha scritto:
>
> > Apparently both oVirt and Kubevirt unconditionally use the stop policy,
> > so I'm afraid in this case we must acknowledge that our expectations
> > don't match reality.
> >
>
> Yeah, of course.
>
> If I understand correctly, not having a pr-manager could mean that QEMU
> > itself is sufficiently privileged and then the same logic would apply.
> >
> > But even if it means that we can't change any persistent reservations
> > from the VM, what use would stopping the VM be? You would run into the
> > exact case I'm describing in the commit message: You try to resume the
> > VM and it immediately stops again because the request still doesn't get
> > through. Or do you expect the host admin to take some manual action
> > then?
> >
>
> Yes, if the PR operation is not allowed then the host admin would probably
> get a notification and release the PR (or perhaps shutdown the VM with an
> error) itself.
>
> And what would you do about the Windows cluster validation case that
> > intentionally sends a request which reservations don't and shouldn't
> > allow? There is nothing on the host side to fix there. The guest is only
> > happy when it gets an error back.
> >
>
> Yes, in that case (which is the most common one) there is nothing you can
> do, so the patch is a good idea even if the case without a PR manager is a
> bit murky.
Ok, so modifying the commit message and removing the 'error'
initialisation it is. Maybe mention the cluster validation case in the
comment here to explain why we do this even for non-pr-manager cases,
but not as a FIXME or TODO because it's not a problem with the
implementation, but we don't have any other choice. Right?
Should I send a v2 for this or is it okay to do this only while applying
the patch?
Kevin
next prev parent reply other threads:[~2024-07-29 16:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-29 9:46 [PATCH 0/4] scsi-block: Fix error handling with r/werror=stop Kevin Wolf
2024-07-29 9:46 ` [PATCH 1/4] scsi-disk: Use positive return value for status in dma_readv/writev Kevin Wolf
2024-07-29 9:46 ` [PATCH 2/4] scsi-block: Don't skip callback for sgio error status/driver_status Kevin Wolf
2024-07-29 9:47 ` [PATCH 3/4] scsi-disk: Add warning comments that host_status errors take a shortcut Kevin Wolf
2024-07-29 9:47 ` [PATCH 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest Kevin Wolf
2024-07-29 11:55 ` Paolo Bonzini
2024-07-29 12:20 ` Kevin Wolf
2024-07-29 12:26 ` Paolo Bonzini
2024-07-29 16:55 ` Kevin Wolf [this message]
2024-07-30 6:06 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZqfJcz6GTsWglrQ5@redhat.com \
--to=kwolf@redhat.com \
--cc=fam@euphon.net \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).