qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Zhao Liu <zhao1.liu@intel.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	"Daniel P. Berrange" <berrange@redhat.com>,
	Eduardo Habkost <eduardo@habkost.net>,
	qemu-devel@nongnu.org, Junjie Mao <junjie.mao@hotmail.com>
Subject: Re: [Question] What is the definition of “private” fields in QOM?
Date: Tue, 22 Oct 2024 23:01:29 +0800	[thread overview]
Message-ID: <Zxe+SZmtJ7XJ49IY@intel.com> (raw)
In-Reply-To: <CAFEAcA9V0yUCOkAWGumoOD_SMk-saS4OU5gL67gj7SRT0v4cPg@mail.gmail.com>

On Mon, Oct 21, 2024 at 05:47:05PM +0100, Peter Maydell wrote:
> Date: Mon, 21 Oct 2024 17:47:05 +0100
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: Re: [Question] What is the definition of “private” fields in
>  QOM?
> 
> On Mon, 21 Oct 2024 at 16:25, Zhao Liu <zhao1.liu@intel.com> wrote:
> > My initial confusion stemmed from seeing the private comment and
> > noticing that there are many direct accesses to parent_obj/parent_class
> > in QEMU (which I could list in my reply to Daniel). Now I understand
> > that these examples are only valid within the class/object
> > implementation or in QOM code.
> >
> > For instance, an example in KVM is a misuse:
> >
> > accel/kvm/kvm-all.c:4285:                        cpu->parent_obj.canonical_path,
> > accel/kvm/kvm-all.c:4377:            if (!apply_str_list_filter(cpu->parent_obj.canonical_path, targets)) {
> >
> >
> > At the same time, I’ve been thinking that the current C implementation
> > seems to have no way to prevent such misuse.
> 
> Mmm. We rely on code review to catch major misuses (and let
> some minor misuses slide because we don't care enough to put
> in the effort to provide a proper API to access the information
> or because we don't want the performance overhead of a QOM cast).
> 
> In a previous iteration of QEMU's design the device's state
> struct was purely private to the implementation source file,
> and code that used the device always did so via a pointer.
> But at some point we decided we wanted to allow users to
> embed the device struct inside their own device struct, which
> needs them to have access to the struct definition (though
> strictly they only need to know the size and alignment
> requirements of it).

Thank you for taking me through this history, I see the intent of
designing the embedded device structure in this way!

> I did a decade ago write a proof-of-concept for marking
> fields in the C struct as "private" such that you could get
> a compile error for touching them:
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01846.html
> which (mis?)uses GCC's __attribute__((deprecated("reason")))
> to arrange that touching the struct field from outside the
> implementation is a compile error. But we never took up the idea.

Very cool! May I ask a few more questions? :-) The feedback on this
series looks very positive, and it seems you were almost close to merge
it at the time. What ultimately led you to decide against it? If we
revisit the issue of Rust's private/pub visibility, I'm curious if we
would face the same dilemma again.

Regards,
Zhao



  parent reply	other threads:[~2024-10-22 14:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-19 16:10 [Question] What is the definition of “private” fields in QOM? Zhao Liu
2024-10-21  8:35 ` Daniel P. Berrangé
2024-10-21  9:22   ` Zhao Liu
2024-10-21  9:25 ` Peter Maydell
2024-10-21 14:22   ` Zhao Liu
2024-10-21 14:20     ` Peter Maydell
2024-10-21 15:18       ` Zhao Liu
2024-10-21 15:06         ` Peter Maydell
2024-10-21 15:41           ` Zhao Liu
2024-10-21 16:47             ` Peter Maydell
2024-10-22  3:08               ` Junjie Mao
2024-10-22  8:42                 ` Peter Maydell
2024-10-22 15:09                   ` Zhao Liu
2024-10-22 15:01               ` Zhao Liu [this message]
2024-10-21 18:46   ` BALATON Zoltan

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=Zxe+SZmtJ7XJ49IY@intel.com \
    --to=zhao1.liu@intel.com \
    --cc=berrange@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=junjie.mao@hotmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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).