all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "DERUMIER, Alexandre via pve-devel" <pve-devel@lists.proxmox.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>,
	"f.gruenbichler@proxmox.com" <f.gruenbichler@proxmox.com>
Cc: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Subject: Re: [pve-devel] [PATCH v4 qemu-server 04/11] blockdev: vm_devices_list : fix block-query
Date: Fri, 11 Apr 2025 17:32:03 +0000	[thread overview]
Message-ID: <mailman.956.1744392763.359.pve-devel@lists.proxmox.com> (raw)
In-Reply-To: <968466724.4390.1743581440569@webmail.proxmox.com>

[-- Attachment #1: Type: message/rfc822, Size: 15288 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.gruenbichler@proxmox.com" <f.gruenbichler@proxmox.com>
Subject: Re: [pve-devel] [PATCH v4 qemu-server 04/11] blockdev: vm_devices_list : fix block-query
Date: Fri, 11 Apr 2025 17:32:03 +0000
Message-ID: <ccc880419200cbab52963d8ab876ba3cd1436735.camel@groupe-cyllene.com>

Hi Fabian,



> @@ -3937,11 +3937,12 @@ sub vm_devices_list {
>   $devices_to_check = $to_check;
>      }
>  
> +    #block devices need to be queried at qdev level, as a device
> +    #don't always have a blockdev drive media attached (cdrom for
> example)
>      my $resblock = mon_cmd($vmid, 'query-block');
> -    foreach my $block (@$resblock) {
> - if($block->{device} =~ m/^drive-(\S+)/){
> - $devices->{$1} = 1;
> - }
> +    $resblock = { map { $_->{qdev} => $_ } $resblock->@* };


>>here you map the full thing

> +    foreach my $blockid (keys %$resblock) {
> + $devices->{$blockid} = 1;


>>but you are only interested in the IDs anyway?

>>so you could just do it like above for PCI devices:
>>
>>
>>foreach my $block (@$resblock) {
>>    my $qdev_id = $block->{qdev};
>>    $devices->{$qdev_id} = 1 if $qdev_id;
>>}

>>that way you don't need to loop twice ;)

ok thanks!


>>this now returns more devices than before/different values for some
>>cases? e.g., efidisk0 has a really different qdev ID compared to its
>>device name, the UEFI firmware (pflash0) is also contained even
>>though it is not a drive, and that's just for the first test VM I
>>checked ;)
>>
>>would it maybe make more sense to special-case the CD drive(s) here?

I didn't have converted pflash device yet (I'm currently doing test
with them), as previously the -drive syntax was more a hack (from qemu-
devel discussion I have seen)

so,I need to verify with full blockdev implementation.

(note the vm_devices_list() is only used by hotplug code, to known if a
device is already plugged. so for pflash, they are no impact anyway)




I'll try to send a patch series next week with all your requested
changed.

have a good weekend !

Alexandre

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

  reply	other threads:[~2025-04-11 17:32 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250311102905.2680524-1-alexandre.derumier@groupe-cyllene.com>
2025-03-11 10:28 ` [pve-devel] [PATCH v4 pve-qemu 1/1] add block-commit-replaces option patch Alexandre Derumier via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 qemu-server 01/11] blockdev: cmdline: convert drive to blockdev syntax Alexandre Derumier via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 pve-storage 1/5] qcow2: add external snapshot support Alexandre Derumier via pve-devel
2025-04-01 13:50   ` Fabian Grünbichler
2025-04-02  8:01     ` DERUMIER, Alexandre via pve-devel
     [not found]     ` <0e2cd118f35aa8d4c410d362fea1a1b366df1570.camel@groupe-cyllene.com>
2025-04-02  8:28       ` Fabian Grünbichler
2025-04-03  4:27         ` DERUMIER, Alexandre via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 qemu-server 02/11] blockdev : convert qemu_driveadd && qemu_drivedel Alexandre Derumier via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 pve-storage 2/5] lvmplugin: add qcow2 snapshot Alexandre Derumier via pve-devel
2025-04-01 13:50   ` Fabian Grünbichler
2025-03-11 10:28 ` [pve-devel] [PATCH v4 qemu-server 03/11] replace qemu_block_set_io_throttle with qom-set throttlegroup limits Alexandre Derumier via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 pve-storage 3/5] storage: vdisk_free: remove external snapshots Alexandre Derumier via pve-devel
2025-04-01 13:50   ` Fabian Grünbichler
2025-04-07 11:02     ` DERUMIER, Alexandre via pve-devel
2025-04-07 11:29     ` DERUMIER, Alexandre via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 qemu-server 04/11] blockdev: vm_devices_list : fix block-query Alexandre Derumier via pve-devel
2025-04-02  8:10   ` Fabian Grünbichler
2025-04-11 17:32     ` DERUMIER, Alexandre via pve-devel [this message]
2025-03-11 10:28 ` [pve-devel] [PATCH v4 pve-storage 4/5] lvm: lvrename helper: allow path Alexandre Derumier via pve-devel
2025-04-01 13:50   ` Fabian Grünbichler
2025-03-11 10:28 ` [pve-devel] [PATCH v4 qemu-server 05/11] blockdev: convert cdrom media eject/insert Alexandre Derumier via pve-devel
2025-03-11 10:28 ` [pve-devel] [PATCH v4 pve-storage 5/5] lvm: add lvremove helper Alexandre Derumier via pve-devel
2025-04-01 13:50   ` Fabian Grünbichler
2025-03-11 10:29 ` [pve-devel] [PATCH v4 qemu-server 06/11] blockdev: block_resize: convert to blockdev Alexandre Derumier via pve-devel
2025-03-11 10:29 ` [pve-devel] [PATCH v4 qemu-server 07/11] blockdev: nbd_export: block-export-add : use drive-$id for nodename Alexandre Derumier via pve-devel
2025-03-11 10:29 ` [pve-devel] [PATCH v4 qemu-server 08/11] blockdev: convert drive_mirror to blockdev_mirror Alexandre Derumier via pve-devel
2025-03-11 10:29 ` [pve-devel] [PATCH v4 qemu-server 09/11] blockdev: change aio on target if io_uring is not default Alexandre Derumier via pve-devel
2025-03-11 10:29 ` [pve-devel] [PATCH v4 qemu-server 10/11] blockdev: add backing_chain support Alexandre Derumier via pve-devel
2025-04-02  8:10   ` Fabian Grünbichler
2025-03-11 10:29 ` [pve-devel] [PATCH v4 qemu-server 11/11] qcow2: add external snapshot support Alexandre Derumier via pve-devel
2025-04-02  8:10   ` Fabian Grünbichler
2025-04-03  4:51     ` DERUMIER, Alexandre via pve-devel
2025-04-04 11:31     ` DERUMIER, Alexandre via pve-devel
     [not found]     ` <3e516016a970e52e5a1014dbcd6cf9507581da74.camel@groupe-cyllene.com>
2025-04-04 11:37       ` Fabian Grünbichler
2025-04-04 13:02         ` DERUMIER, Alexandre via pve-devel

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=mailman.956.1744392763.359.pve-devel@lists.proxmox.com \
    --to=pve-devel@lists.proxmox.com \
    --cc=alexandre.derumier@groupe-cyllene.com \
    --cc=f.gruenbichler@proxmox.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal