Re: [PATCH RFC v3 09/16] qemu: hotplug: Support hot attach and detach block disk along with throttle filters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2024/8/20 23:08, Peter Krempa wrote:
On Tue, Aug 20, 2024 at 22:48:53 +0800, Chun Feng Wu wrote:
On 2024/8/19 18:59, Peter Krempa wrote:
On Mon, Aug 19, 2024 at 18:27:46 +0800, Chun Feng Wu wrote:
On 2024/8/16 23:14, Peter Krempa wrote:
On Thu, Aug 15, 2024 at 13:04:47 +0800, Chun Feng Wu wrote:

(I'd suggest you trim irrelevant stuff when responding. It's hard to
find what you've responded to in this massive message .

On 2024/8/9 22:04, Peter Krempa wrote:
On Wed, Jun 12, 2024 at 03:02:17 -0700,wucf@xxxxxxxxxxxxx  wrote:
From: Chun Feng Wu<wucf@xxxxxxxxxxxxx>
[...]

it apply groups on top source(vm1_disk_2.qcow2) only? is there scenario to
apply throttle-groups on backing store source?

     <disk type='file' device='disk'>
         <driver name='qemu' type='qcow2'/>
         <source file='/virt/disks/vm1_disk_2.qcow2' index='1'>
             <throttlefilters>
               <throttlefilter group='limit0'/>
             </throttlefilters>
         </source>
         <backingStore type='file' index='4'>
           <format type='qcow2'/>
           <source file='/virt/disks/backing.qcow2'>
             <throttlefilters>
               <throttlefilter group='limit1'/>
             </throttlefilters>
           </source>
           <backingStore/>
         </backingStore>
         <target dev='vdc' bus='virtio'/>
         <alias name='virtio-disk2'/>
         <address type='pci' domain='0x0000' bus='0x00' slot='0x06'
function='0x0'/>
      </disk>
Yeah, something like this. That allows adding filters on other layers
too.

As I said it depends on how you expect to use this feature, because both
make sense.

I do reckon that in most cases this is an overkill though.

If you decide that this is too complicated, you can use the approach you
did, but then need to store the nodename of the throttle layer on the
disk level in the status XML.
sure, then let's stick to current implementation about filter
chain(per-disk), about status XML, do you mean <privateData> to store
nodename? if so, does the following xml look good to you?

    <throttlefilters>
      <throttlefilter  group='limit2'>
        <privateData>
          <nodename type='throttle-filter'
name='0123456789ABCDEF0123456789ABCDE'/>
        </privateData>
      </throttlefilter>
    </throttlefilters>
I suggest you don't add the privateData sub-element to the
'throttlefilters' as it will be very hard to plumb that in. You'd
require private data callbacks which the XML parser (generic) calls from
the qemu driver (specific) to parse this.

I'd rather suggest you use the disk private data section and format them
there. Use the group name as a key to find them as that is (or at least
should be) unique.

if so, it could be the following xml:

<disk>
   <privateData>
        <nodenames>
            <nodename type='throttle-filter'
name='libvirt-nodenameindex-filter' key='group_name'/>
        </nodenames>
   </privateData>
</disk>
Yes. Although I'd name the 'key' field 'group' instead.

and

struct _qemuDomainDiskPrivate {
...
GHashTable *throttleFilters;
...
}
This shouldn't be needed. You can store the nodenames in the same data
structure you do now. The formatter will format it from the data
structure and parser will fill it into the appropriate
fields based on the group name.

Yes it's O(N2)-ish complexity but N is small, so having a hash
table doesn't make sense.

In fact you don't want a hash table because you need the
filters ordered in the order defined in the XML and a hash table would
re-order them randomly every time.
Thanks for your suggestion! it makes more sense, I am drafting this part in v4

--
Thanks and Regards,

Wu




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux