this change is largely the
same: barring exceptional circumstances, the overall (maintenance,
cognitive) cost of straying from the established norm, now spanning
three existing architectures, likely outweighs the benefits.
--
Andrea Bolognani / Red Hat / Virtualization
On Tue, Feb 06, 2024 at 05:38:35AM -0800, Andrea Bolognani wrote:
> On Tue, Feb 06, 2024 at 10:10:02AM +0800, Xianglai Li wrote:
> > The UEFI loading mode in loongarch is very different
> > from that in other architectures:loongarch's UEFI code
> > is in rom, while other arc
AM, and the cost of loongarch's current loading method
> far outweighs the benefits, so we decided to use the same UEFI loading
> scheme as other architectures.
>
> Cc: Andrea Bolognani
> Cc: maob...@loongson.cn
> Cc: Philippe Mathieu-Daudé
> Cc: Song Gao
> Cc: zhaotian...@
-pflash1-storage"}'
> > > \
> > > -machine
> > > virt,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \
> > >
> > >
> > > -------
> > >
> > > This way, without modifying libvirt, QEMU_CODE.fd can be loaded into the
> > > rom,
> > >
> > > but it is still a little strange that it is clearly rom but set a
> > > "pflash0"
> > > attribute, which can be confusing.
> >
> > We recently had a very similar discussion regarding EFI booting on
> > RISC-V.
> >
> > Personally I would prefer to see the approach with two pflash
> > devices, one read-only and one read/write, adopted. This is pretty
> > much the de-facto standard across architectures: x86_64, aarch64 and
> > riscv64 all boot edk2 this way.
> >
> > I understand the desire to simplify things where possible, and I am
> > sympathetic towards it. If we could boot from just rom, without using
> > pflash at all, I would definitely see the appeal. However, as noted
> > earlier, in the case of EFI having some read/write storage space is
> > necessary to expose the full functionality, so going without it is
> > not really an option.
> >
> > With all the above in mind, the cost of loongarch64 doing things
> > differently from other architectures seems like it would outweight
> > the benefits, and I strongly advise against it.
>
> Hi Andrea :
>
> So, just to be clear, you're not suggesting either of the options I
> suggested above,
>
> are you? And still recommend that we use a two-piece pflash solution
> similar to other architectures,
>
> right?
I thought that the second solution that you proposed above was the
same as other architectures, but reading again I think what you're
suggesting is that you'd have some logic in QEMU that ensures what
you ask to be loaded in pflash0 (CODE) would be loaded into ROM, and
what you ask to be loaded in pflash1 (VARS) would instead go into the
single pflash device for the machine?
If that's a correct understanding, my vote is emphatically against
that approach. The QEMU interface should be as "do exactly as I said"
as possible, with little to no cleverness to it. This is especially
true for a young architecture such as loongarch64, where you don't
need to be concerned about causing breakages for all the users
accumulated over decades.
> Hi Philippe :
>
> I look forward to your reply and the comments of other members of the qemu
> community very much.
>
> If everyone has no opinions,
>
> I will submit a patch to the community to change the loading mode of qemu
> under loongarch architecture to UEFI with two pieces of pflash.
I think this is the way to go.
Note that changing this will likely requires adaptations in edk2 too.
That was the case when riscv64 made the same change.
--
Andrea Bolognani / Red Hat / Virtualization
dk2/loongarch64/QEMU_VARS.fd/","node-name":"libvirt-pflash0-storage","auto-read-only":false,"discard":"unmap"}'
> \
> -blockdev
> '{"node-name":"libvirt-pflash0-format","read-only":false,"driver":"raw","file":"libvirt-pflash0-storage"}'
> \
> -machine virt,pflash=libvirt-pflash0-format \
> -bios /usr/share/edk2/loongarch64///QEMU_EFI.fd \
>
> ---
>
>Option 2:
>
> Solution 2 mainly starts from qemu. Now the rom that bios is loaded into is
> a memory region that cannot be configured with attributes,
>
> so we imagine abstracting rom as a device, creating it during machine
> initialization and setting "pflash0" attribute for it.
>
> Then create a pflash and set its property to "pflash1", so our startup
> command will look like this:
>
>
> ---
>
> -blockdev
> '{"driver":"file","filename":"/usr/share/edk2/loongarch64/QEMU_EFI.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
> \
> -blockdev
> '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
> \
> -blockdev
> '{"driver":"file","filename":"/usr/share/edk2/loongarch64/QEMU_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
> \
> -blockdev
> '{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
> \
> -machine virt,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format \
>
>
> ---
>
> This way, without modifying libvirt, QEMU_CODE.fd can be loaded into the
> rom,
>
> but it is still a little strange that it is clearly rom but set a "pflash0"
> attribute, which can be confusing.
We recently had a very similar discussion regarding EFI booting on
RISC-V.
Personally I would prefer to see the approach with two pflash
devices, one read-only and one read/write, adopted. This is pretty
much the de-facto standard across architectures: x86_64, aarch64 and
riscv64 all boot edk2 this way.
I understand the desire to simplify things where possible, and I am
sympathetic towards it. If we could boot from just rom, without using
pflash at all, I would definitely see the appeal. However, as noted
earlier, in the case of EFI having some read/write storage space is
necessary to expose the full functionality, so going without it is
not really an option.
With all the above in mind, the cost of loongarch64 doing things
differently from other architectures seems like it would outweight
the benefits, and I strongly advise against it.
--
Andrea Bolognani / Red Hat / Virtualization
ibexpat1 libpython3_11-1_0 python311 python311-base
4 new packages to install.
Overall download size: 12.8 MiB. Already cached: 0 B.
After the operation, additional 50.4 MiB will be used.
This is the main reason why we never really bothered trying to avoid
it.
Hope this helps!
--
Andrea Bolognani / Red Hat / Virtualization
lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index 82092c9f17..149b15de57 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -97,6 +97,7 @@ packages:
- python3-pip
- python3-sphinx
- python3-sphinx-rtd-theme
+ - python3-sqlite3
- python3-tomli
- python3-venv
- rpm2cpio
--
Andrea Bolognani / Red Hat / Virtualization
On Fri, May 26, 2023 at 11:10:12AM +0200, Andrew Jones wrote:
> On Fri, May 26, 2023 at 04:42:57AM -0400, Andrea Bolognani wrote:
> > On Fri, May 26, 2023 at 10:34:36AM +0200, Andrew Jones wrote:
> > > On Fri, May 26, 2023 at 03:49:11AM -0400, Andrea Bolognani wrote:
> >
On Thu, Nov 09, 2023 at 08:01:48PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 09:34:56AM -0800, Andrea Bolognani wrote:
> > We should call this Null instead of IsNull, and also make it a
> > pointer similarly to what I just suggested for union branches
> > that don't
On Thu, Nov 09, 2023 at 07:35:04PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 09:29:28AM -0800, Andrea Bolognani wrote:
> > Additionally, this would allow client code that *looks* at the
> > union to keep working even if actual data is later added to the
> >
On Thu, Nov 09, 2023 at 08:13:38PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 09:59:50AM -0800, Andrea Bolognani wrote:
> > Now, I'm not sure I would go as far as suggesting that the
> > GetName() function should be completely removed, but maybe we
> > can tr
On Thu, Nov 09, 2023 at 08:31:01PM +0100, Victor Toso wrote:
> On Thu, Nov 09, 2023 at 10:24:20AM -0800, Andrea Bolognani wrote:
> > On Mon, Oct 16, 2023 at 05:27:03PM +0200, Victor Toso wrote:
> > > Example:
> > > qapi:
> > > | { 'comman
it priority.
Overall, I think that the current API is quite close to being a solid
PoC that can be used as a starting point for further development.
--
Andrea Bolognani / Red Hat / Virtualization
e QueryAudiodevsCommandReturn struct {
MessageId string `json:"id,omitempty"`
Error *QAPIError `json:"error,omitempty"`
Result[]Audiodev `json:"return"`
}
when the return type is an array. Is that the correct behavior? I
haven't thought too hard about it, but it seems odd so I though I'd
bring it up.
--
Andrea Bolognani / Red Hat / Virtualization
(Event, error) {
type event struct {
Name string `json:"event"`
}
tmp := event{}
if err := json.Unmarshal(data, ); err != nil {
return nil, err
}
switch tmp.Name {
case "MEMORY_DEVICE_SIZE_CHANGE":
return {}, nil
...
}
type StrOrNull struct {
> | S *string
> | IsNull bool
> | }
We should call this Null instead of IsNull, and also make it a
pointer similarly to what I just suggested for union branches that
don't have additional data attached to them.
--
Andrea Bolognani / Red Hat / Virtualization
itionally, this would allow client code that *looks* at the union
to keep working even if actual data is later added to the branch;
client code that *creates* the union would need to be updated, of
course, but that would be the case regardless.
--
Andrea Bolognani / Red Hat / Virtualization
On Mon, Nov 06, 2023 at 04:52:12PM +0100, Victor Toso wrote:
> On Mon, Nov 06, 2023 at 07:28:04AM -0800, Andrea Bolognani wrote:
> > On a partially related note: while I haven't yet looked closely at
> > how much effort you've dedicated to producing pretty output, from a
ed closely at
how much effort you've dedicated to producing pretty output, from a
quick look at generate_struct_type() it seems that the answer is "not
zero". I think it would be fine to simplify things there and produce
ugly output, under the assumption that gofmt will be called on the
generated code immediately afterwards. The C generator doesn't have
this luxury, but we should take advantage of it.
--
Andrea Bolognani / Red Hat / Virtualization
On Tue, Oct 31, 2023 at 09:42:10AM -0700, Andrea Bolognani wrote:
> On Fri, Oct 27, 2023 at 07:33:30PM +0200, Victor Toso wrote:
> > Hi,
> >
> > Daniel & Andrea, it would be great to have your take on the Go
> > side of this series. If we can agree with an acceptab
ent code meet your expectations.
Apologies for not providing any feedback so far. I'll do my best to
get around to it by the end of the week.
--
Andrea Bolognani / Red Hat / Virtualization
ibvirt? We use '-display
none' for every single VM we run.
--
Andrea Bolognani / Red Hat / Virtualization
On Thu, Oct 26, 2023 at 05:14:49PM +0200, Andrew Jones wrote:
> On Thu, Oct 26, 2023 at 07:36:21AM -0700, Andrea Bolognani wrote:
> > On Mon, Oct 23, 2023 at 07:35:16PM +0200, Andrew Jones wrote:
> > > On Mon, Oct 23, 2023 at 02:00:00PM -0300, Daniel Henrique Barboza wrote:
>
>
> > > -cpu rv64,rva22u64=true,rva22u64=false,c=off
> > >
> > > should not warn (rva22u64 is not enabled)
I think these should be hard errors, not warnings.
If you're enabling a profile and then disabling an extension that's
mandatory for that profile, you've invalidated the profile. You've
asked for a configuration that doesn't make any sense: you can't have
a CPU that both implements a profile and lacks one of its mandatory
extensions.
QEMU users could easily miss the warning. libvirt users won't see it
at all. It's a user error and it needs to be treated as such IMO.
--
Andrea Bolognani / Red Hat / Virtualization
decisions made:
>
> - disabling a profile flag does nothing, i.e. we won't mass disable
> mandatory extensions of the rva22U64 profile if the user sets
> rva22u64=false;
Wouldn't it make more sense to error out when this is requested?
Silently ignoring an explicit request mad
other args
For TCG guests only, it is also possible to boot M-mode firmware from
the first flash device (pflash0) by additionally passing ``-bios
none``, as in
..code-block:: bash
$ qemu-system-riscv64 \
-bios none \
-blockdev node-name=pflash0,driver=file,read-only=on,filename=
pplied, pflash0 is assumed to contain S-mode
code *unless* you go out of your way and add -bios none to the
command line.
It seems to me that this default behavior will work fine for KVM
guests too, based on what you wrote above. Isn't that the case?
--
Andrea Bolognani / Red Hat / Virtualization
capable hardware for now!
What's important is that you don't get a different MTE version than
what you asked for. I assume that the existing KVM API for enabling
MTE have good enough granularity to make this work? If not, that's
going to be a problem :)
--
Andrea Bolognani / Red Hat / Virtualization
based on Alistair's riscv-to-apply.next branch.
>
> Changes since v4:
> 1) Updated patch 2 to avoid accessing private field as per feedback
> from Philippe.
> 2) Updated documentation patch to add read-only for ROM usage.
> 3) Rebased to latest riscv-to-apply.ne
On Fri, May 26, 2023 at 10:34:36AM +0200, Andrew Jones wrote:
> On Fri, May 26, 2023 at 03:49:11AM -0400, Andrea Bolognani wrote:
> > So, are edk2 users the only ones who would (temporarily) need to
> > manually turn ACPI off if virt-manager started enabling it by
> > def
On Fri, May 26, 2023 at 08:39:07AM +0200, Andrew Jones wrote:
> On Thu, May 25, 2023 at 11:03:52AM -0700, Andrea Bolognani wrote:
> > With these patches applied, libvirt built from the master branch,
> > edk2 built from your branch and a JSON firmware descriptor for it
> >
ng developers convenient access to a reasonable RISC-V
environment they can play around with!
Thanks a lot for your work on this, and I can't wait to see it
all merged :)
Tested-by: Andrea Bolognani
--
Andrea Bolognani / Red Hat / Virtualization
50-edk2-riscv64-raw.json
Description: application/json
On Thu, May 25, 2023 at 08:32:46PM +0530, Sunil V L wrote:
> On Thu, May 25, 2023 at 01:43:28PM +0000, Andrea Bolognani wrote:
> > I have also tried booting an openSUSE Tumbleweed "JeOS" image, since
> > that's the only distro I'm aware of that uses UEFI boot on RISC-V a
On Wed, May 24, 2023 at 11:03:52PM +0530, Sunil V L wrote:
> On Wed, May 24, 2023 at 03:50:34PM +0000, Andrea Bolognani wrote:
> > First off, the only RISC-V edk2 build readily accessible to me (from
> > the edk2-riscv64 Fedora package) is configured to work off a R/W
> &
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
-machine virt,pflash0=libvirt-pflash0-format
instead, which both aarch64/virt and x86_64/q35 machine types ar
seems to be rough consensus
of that not being the case here.
So, can we get v1 merged?
--
Andrea Bolognani / Red Hat / Virtualization
tly packaged for Fedora, doesn't seem to have separate CODE and
VARS files:
$ ls /usr/share/edk2/riscv/* | cat
/usr/share/edk2/riscv/RISCV_VIRT.fd
/usr/share/edk2/riscv/RISCV_VIRT.raw
Is that something that needs to be addressed in upstream edk2? If so,
will you be looking into it?
--
Andrea Bol
On Thu, May 18, 2023 at 02:53:05PM +1000, Alistair Francis wrote:
> On Wed, May 17, 2023 at 6:45 PM Andrea Bolognani wrote:
> > On Wed, May 17, 2023 at 02:57:12PM +1000, Alistair Francis wrote:
> At one point we loaded Oreboot in in flash and booted from that. I
> think Oreb
with using a R/O pflash for CODE
as opposed to -bios? What makes that approach so problematic? You're
still going to need to use pflash for VARS anyway...
--
Andrea Bolognani / Red Hat / Virtualization
On Wed, May 17, 2023 at 02:57:12PM +1000, Alistair Francis wrote:
> On Mon, May 8, 2023 at 9:45 PM Andrea Bolognani wrote:
> > > > Taking a step back, what is even the use case for having M-mode code
> > > > in pflash0? If you want to use an M-mode firmware, can't
On Mon, May 08, 2023 at 04:53:46PM +0530, Sunil V L wrote:
> On Mon, May 08, 2023 at 03:00:02AM -0700, Andrea Bolognani wrote:
> > I think that it's more important to align with other architectures.
> >
> > The number of people currently running edk2 on RISC-V is probably
e would be to go back to v1.
> Second, since unit 0 for RISC-V is currently assumed to start in M-mode fw
> which is secure, I think it makes sense to keep variables also in unit
> 0.
If you're storing variables rather than code in pflash0, does it even
make sense to talk about M-mode and S-mode?
Taking a step back, what is even the use case for having M-mode code
in pflash0? If you want to use an M-mode firmware, can't you just use
-bios instead? In other words, can we change the behavior so that
pflash being present always mean loading S-mode firmware off it?
--
Andrea Bolognani / Red Hat / Virtualization
On Fri, Apr 21, 2023 at 11:31:44PM +0200, Philippe Mathieu-Daudé wrote:
> On 21/4/23 18:48, Andrea Bolognani wrote:
> > For what it's worth, this change seems to go in the right direction
> > by making things similar to other architectures (x86, Arm) so I'd
> >
ote that RISC-V doesn't have
versioned machine types yet, so this kind of breakage is not
necessarily unexpected.
>From libvirt's point of view, being able to detect whether the new
behavior is implemented by looking for some machine type property
would be enough to handle the transition smoothly. That would of
course not help people running QEMU directly.
For what it's worth, this change seems to go in the right direction
by making things similar to other architectures (x86, Arm) so I'd
love to see it happen.
--
Andrea Bolognani / Red Hat / Virtualization
On Thu, Mar 02, 2023 at 02:26:06PM +0100, Cornelia Huck wrote:
> On Wed, Mar 01 2023, Andrea Bolognani wrote:
> > Note that, from libvirt's point of view, there's no advantage to
> > doing things that way instead of what you already have. Handling the
> > additional machine p
On Wed, Mar 01, 2023 at 03:15:17PM +0100, Cornelia Huck wrote:
> On Wed, Mar 01 2023, Andrea Bolognani wrote:
> > I'm actually a bit confused. The documentation for the mte property
> > of the virt machine type says
> >
> > mte
> > Set on/off to enable/di
On Wed, Mar 01, 2023 at 11:17:40AM +0100, Cornelia Huck wrote:
> On Tue, Feb 28 2023, Andrea Bolognani wrote:
> > On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
> >> +MTE CPU Property
> >> +
> >> +
> >> +The ``mte`` p
hange in a future QEMU version.
If and when this changes, we should ensure that the new default
behavior doesn't affect existing machine types, otherwise we will
break guest ABI for existing VMs.
--
Andrea Bolognani / Red Hat / Virtualization
On Tue, Feb 07, 2023 at 08:20:31PM +0100, Andrew Jones wrote:
> On Tue, Feb 07, 2023 at 06:38:15AM -0800, Andrea Bolognani wrote:
> > Well, it looks like -no-acpi will come for free after all, thanks to
> > the code you pasted above, as long as the machine property follows
>
On Tue, Feb 07, 2023 at 03:02:19PM +0100, Thomas Huth wrote:
> On 07/02/2023 14.56, Andrea Bolognani wrote:
> > It looks like i440fx and q35 both have an 'acpi' machine property. Is
> > -no-acpi just sugar for acpi=off?
>
> Yes, it is, see softmmu/vl.c:
>
>
On Tue, Feb 07, 2023 at 11:23:53AM +0100, Andrew Jones wrote:
> On Mon, Feb 06, 2023 at 12:18:06PM +0100, Philippe Mathieu-Daudé wrote:
> > On 6/2/23 11:54, Andrea Bolognani wrote:
> > > On Thu, Feb 02, 2023 at 10:22:15AM +0530, Sunil V L wrote:
> > > > +objec
operty_set_description(oc, "acpi",
> + "Enable ACPI");
The way this works on other architectures (x86_64, aarch64) is that
you get ACPI by default and can use -no-acpi to disable it if
desired. Can we have the same on RISC-V, for consistency?
--
Andrea Bolognani / Red Hat / Virtualization
le changed, 10 insertions(+), 2 deletions(-)
FYI libvirt will already reject any attempts to place the device
anywhere but directly on pcie.0, so from our point of view merging
this patch is perfectly fine.
--
Andrea Bolognani / Red Hat / Virtualization
significantly smaller file
that can still serve the intended purpose.
--
Andrea Bolognani / Red Hat / Virtualization
On Wed, Aug 17, 2022 at 04:04:19PM +0200, Victor Toso wrote:
> On Tue, Jul 05, 2022 at 08:45:06AM -0700, Andrea Bolognani wrote:
> > On Fri, Jun 17, 2022 at 02:19:26PM +0200, Victor Toso wrote:
> > > func (s *BlockdevRef) UnmarshalJSON(data []byte) error {
> > > /
On Fri, Aug 19, 2022 at 09:20:52AM +0200, Victor Toso wrote:
> > > On Wed, Jul 06, 2022 at 10:37:54AM +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Jul 06, 2022 at 04:28:16AM -0500, Andrea Bolognani wrote:
> > > > > You're right, that is un
On Tue, Jun 14, 2022 at 06:42:58AM -0700, Andrea Bolognani wrote:
> On Wed, May 04, 2022 at 09:23:28AM +0100, Daniel P. Berrangé wrote:
> > On Wed, May 04, 2022 at 01:01:03AM -0700, Andrea Bolognani wrote:
> > > On Wed, Apr 20, 2022 at 09:18:47AM -0700, Andrea Bolognani wrote:
&
On Wed, Jul 06, 2022 at 04:07:43PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 06, 2022 at 09:53:43AM -0500, Andrea Bolognani wrote:
> > Yeah but we're generating structs for all possible events ourselves
> > and we don't really expect external implementations of this
> >
On Tue, Jul 05, 2022 at 05:47:25PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 05, 2022 at 08:45:54AM -0700, Andrea Bolognani wrote:
> > On Fri, Jun 17, 2022 at 02:19:29PM +0200, Victor Toso wrote:
> > > func (s *AcpiDeviceOstEvent) GetTimestamp() Timestamp {
> > >
On Wed, Jul 06, 2022 at 10:48:06AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 06, 2022 at 10:37:54AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Jul 06, 2022 at 04:28:16AM -0500, Andrea Bolognani wrote:
> > > func (s *GuestPanicInformation) UnmarshalJSON(data []byte) error
On Tue, Jul 05, 2022 at 05:35:26PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 05, 2022 at 08:45:30AM -0700, Andrea Bolognani wrote:
> > All this string manipulation looks sketchy. Is there some reason that
> > I'm not seeing preventing you for doing something like the untested
nt code, in the commit message you have
> input := `{"event":"MEMORY_DEVICE_SIZE_CHANGE",` +
> `"timestamp":{"seconds":1588168529,"microseconds":201316},` +
>
> `"data":{"id":"vm0","size":1073741824,"qom-path":"/machine/unattached/device[2]"}}`
> e, err := UnmarshalEvent([]byte(input)
> if err != nil {
> panic(err)
> }
> if e.GetName() == `MEMORY_DEVICE_SIZE_CHANGE` {
> m := e.(*MemoryDeviceSizeChangeEvent)
> // m.QomPath == "/machine/unattached/device[2]"
> }
I don't think we should encourage the use of string comparison for
the purpose of going from a generic Event instance to a specific one:
a better way would be to use Go's type switch feature, such as
switch m := e.(type) {
case *MemoryDeviceSizeChangeEvent:
// m.QomPath == "/machine/unattached/device[2]"
}
In fact, I don't really see a reason to provide the Name() getter
outside of possibly diagnostics, and given the potential of it being
misused I would argue that it might be better not to have it.
--
Andrea Bolognani / Red Hat / Virtualization
Just like Event.GetName() and Command.GetName(), I'm not convinced we
should have this.
Of course, all the comments about how marshalling and unmarshalling
are handled made for events also apply here.
--
Andrea Bolognani / Red Hat / Virtualization
should match more closely that of the QAPI schema, so e.g.
block-related types should all go in one file, net-related types in
another one and so on.
Looking forward to the next iteration :)
--
Andrea Bolognani / Red Hat / Virtualization
gt; if err := json.Unmarshal(data, s.S390); err != nil {
> s.S390 = nil
> return err
> }
> }
> // Unrecognizer drivers are silently ignored.
> return nil
This looks pretty reasonable, but since you're only using "peek" to
look at the discriminator you should be able to leave out the Alias
type entirely and perform the initial Unmarshal operation while
ignoring all other fields.
The comments made elsewhere about potentially being more strict and
rejecting invalid JSON also apply here.
--
Andrea Bolognani / Red Hat / Virtualization
ht be valid JSON that's been
produced by a version of QEMU that introduced additional options to
the alternate. In the spirit of "be liberal in what you accept", I
think we should probably not reject that? Of course then the client
code will have to look like
if r.Definition !=
On Wed, May 04, 2022 at 09:23:28AM +0100, Daniel P. Berrangé wrote:
> On Wed, May 04, 2022 at 01:01:03AM -0700, Andrea Bolognani wrote:
> > On Wed, Apr 20, 2022 at 09:18:47AM -0700, Andrea Bolognani wrote:
> > > On Wed, Apr 20, 2022 at 05:15:08PM +0100, Daniel P. Berrangé wro
L all over
it and get it merged.
Basically get the MVP done and then iterate over it in-tree rather
than trying to get everything perfect from the start.
Sounds reasonable?
--
Andrea Bolognani / Red Hat / Virtualization
ot;
machine type, can we just take the current virt machine type and
rename it to something else? And have your simpler code take over the
virt name? Sure, it will cause some pain in the short term, but the
RISC-V ecosystem is still young enough for it to not be a deal
breaker.
--
Andrea Bolognani / Red Hat / Virtualization
{ // really BlockResizeArguments520
Device: device,
Size: size,
}
or even
if !qmp.HasAPI(qmp.API.Latest) {
panic("update QEMU")
}
cmd := {
NodeName: nodeName,
Size: size,
}
Should be easy enough to achieve with type aliases.
--
Andrea Bolognani / Red Hat / Virtualization
}
This honestly looks pretty unwieldy.
If the application already knows it's targeting a specific version of
the QEMU API, which for the above code to make any sense it will have
to, couldn't it do something like
import qemu .../qemu/v700
at the beginning of the file and then use regular o
On Mon, May 09, 2022 at 12:21:10PM +0200, Victor Toso wrote:
> On Tue, Apr 19, 2022 at 11:12:28AM -0700, Andrea Bolognani wrote:
> > Based on the example you have in the README and how commands are
> > defined, invoking (a simplified version of) the trace-event-get-state
> &g
On Tue, May 10, 2022 at 10:52:34AM +0100, Daniel P. Berrangé wrote:
> On Mon, May 02, 2022 at 10:01:41AM -0400, Andrea Bolognani wrote:
> > Revised proposal for the annotation:
> >
> > ns:word-WORD-WoRD-123Word
>
> Ugly, but we should only need this in the fairly ni
On Wed, Apr 20, 2022 at 09:18:47AM -0700, Andrea Bolognani wrote:
> On Wed, Apr 20, 2022 at 05:15:08PM +0100, Daniel P. Berrangé wrote:
> > On Wed, Apr 20, 2022 at 06:03:11PM +0200, Andrea Bolognani wrote:
> > > These changes match those made in the followi
On Tue, May 03, 2022 at 10:44:37AM +0200, Markus Armbruster wrote:
> I posted a more complete fix as "[PATCH] qapi: Fix malformed "Since:"
> section tags", and you even reviewed it :)
Oh boy, I clearly need more coffee. Sorry for the noise :/
--
Andrea Bolognani / Red Hat / Virtualization
On Tue, May 03, 2022 at 09:57:27AM +0200, Markus Armbruster wrote:
> Andrea Bolognani writes:
> > I still feel that 1) users of a language SDK will ideally not need to
> > look at the QAPI schema or wire chatter too often
>
> I think the most likely point of contact is t
This only affects readability. The generated documentation
doesn't change.
Signed-off-by: Andrea Bolognani
Reviewed-by: Markus Armbruster
---
qapi/block-core.json | 5 +
qapi/block.json | 1 +
qapi/crypto.json | 7 +++
qapi/machine.json| 1 +
qapi/migration.json | 4
The missing colon causes them to be interpreted as regular
text.
Signed-off-by: Andrea Bolognani
---
qapi/crypto.json | 2 +-
qapi/misc.json | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 1ec54c15ca..529aa730d4 100644
On Mon, May 02, 2022 at 07:24:53PM +0200, Markus Armbruster wrote:
> Andrea Bolognani writes:
> > On Mon, May 02, 2022 at 10:50:07AM +0200, Markus Armbruster wrote:
> >> I doubt changing to a different alignment now is useful. The next
> >> patch, however, drops the al
Signed-off-by: Andrea Bolognani
Reviewed-by: Markus Armbruster
---
qapi/audio.json | 1 -
qapi/block-core.json | 11 ---
qapi/block.json | 3 ---
qapi/char.json | 1 -
qapi/control.json| 1 -
qapi/crypto.json | 12
qapi
It should start on the very first column.
Signed-off-by: Andrea Bolognani
Reviewed-by: Markus Armbruster
---
qapi/ui.json | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 059302a5ef..43e62efd76 100644
If patch 8/8 is accepted, 7/8 should be squashed into it to reduce
churn.
Changes from [v1]:
* use a more fine-grained split for whitespace changes.
[v1] https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg05406.html
Andrea Bolognani (8):
qapi: Drop stray trailing symbol
qapi: Fix
Signed-off-by: Andrea Bolognani
Reviewed-by: Markus Armbruster
---
qapi/block-core.json | 4
qapi/block.json | 1 -
qapi/char.json| 1 -
qapi/common.json | 2 --
qapi/control.json | 2 --
qapi/crypto.json | 1 -
qapi/machine.json | 2 --
qapi
Perfectly aligned things look pretty, but keeping them that
way as the schema evolves requires churn, and in some cases
newly-added lines are not aligned properly.
Overall, trying to align things is just not worth the trouble.
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 43
Use the minimum number of spaces necessary.
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 38 +++---
qapi/control.json| 10 +-
qapi/crypto.json | 42 +-
qapi/ui.json | 16
The only instances that get changed are those in which the
additional whitespace was not (or couldn't possibly be) used for
alignment purposes.
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 24
qapi/block-export.json | 2 +-
qapi/block.json| 2
Signed-off-by: Andrea Bolognani
Reviewed-by: Markus Armbruster
---
qapi/run-state.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 8124220bd9..15d6c9a2ed 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -348,7
ree, something else... ?
What about the changes you suggested to the commit message of 3/7?
--
Andrea Bolognani / Red Hat / Virtualization
On Mon, May 02, 2022 at 10:50:07AM +0200, Markus Armbruster wrote:
> Andrea Bolognani writes:
> > -# @writeback: true if writeback mode is enabled
> > -# @direct: true if the host page cache is bypassed (O_DIRECT)
> > -# @no-flush:true if flush requests are i
On Mon, May 02, 2022 at 01:46:23PM +0200, Markus Armbruster wrote:
> Andrea Bolognani writes:
> >> > The wire protocol would still retain the unappealing name, but at
> >> > least client libraries could hide the uglyness from users.
> >>
> >> At the pr
On Mon, May 02, 2022 at 09:21:36AM +0200, Markus Armbruster wrote:
> Andrea Bolognani writes:
> > The wire protocol would still retain the unappealing name, but at
> > least client libraries could hide the uglyness from users.
>
> At the price of mild inconsistency between
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 4
qapi/block.json | 1 -
qapi/char.json| 1 -
qapi/common.json | 2 --
qapi/control.json | 2 --
qapi/crypto.json | 1 -
qapi/machine.json | 2 --
qapi/migration.json | 7 ---
qapi
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 5 +
qapi/block.json | 1 +
qapi/crypto.json | 7 +++
qapi/machine.json| 1 +
qapi/migration.json | 4
5 files changed, 18 insertions(+)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index
The last patch could very reasonably be squashed into the previous
one, but since the changes could be considered more controversial I
thought it would be better if the two could be reviewed and judged
separately.
Andrea Bolognani (7):
qapi: Drop stray trailing symbol
qapi: Fix comment
Vertical alignment is sacrificed in the process.
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 43 +--
qapi/block.json | 6 +++---
qapi/char.json | 6 +++---
qapi/control.json| 6 +++---
qapi/crypto.json | 32
Care was taken not to break vertical alignment.
Signed-off-by: Andrea Bolognani
---
qapi/block-core.json | 62 +-
qapi/block-export.json | 2 +-
qapi/block.json| 2 +-
qapi/char.json | 2 +-
qapi/control.json | 10 +++
qapi
Signed-off-by: Andrea Bolognani
---
qapi/audio.json | 1 -
qapi/block-core.json | 11 ---
qapi/block.json | 3 ---
qapi/char.json | 1 -
qapi/control.json| 1 -
qapi/crypto.json | 12
qapi/job.json| 1 -
qapi
Signed-off-by: Andrea Bolognani
---
qapi/run-state.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 8124220bd9..15d6c9a2ed 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -348,7 +348,7 @@
#
# @poweroff
It should start on the very first column.
Signed-off-by: Andrea Bolognani
---
qapi/ui.json | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 059302a5ef..43e62efd76 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
On Thu, Apr 28, 2022 at 03:50:55PM +0200, Markus Armbruster wrote:
> Andrea Bolognani writes:
> > One concern that I have is about naming struct members: things like
> > SpiceInfo.MouseMode and most others are translated from the QAPI
> > schema exactly the way you'd expect
1 - 100 of 381 matches
Mail list logo
