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

Re: [libvirt] [PATCH v4 2/5] qemu monitor: add multithread compress parameters accessors



On Wed, Mar 23, 2016 at 15:34:07 +0300, Nikolay Shirokovskiy wrote:
> >> +typedef struct _qemuMonitorMigrationParameters qemuMonitorMigrationParameters;
> >> +typedef qemuMonitorMigrationParameters *qemuMonitorMigrationParametersPtr;
> >> +struct _qemuMonitorMigrationParameters {
> >> +    unsigned int level_set : 1;
> >> +    unsigned int threads_set : 1;
> >> +    unsigned int dthreads_set : 1;
> >> +
> >> +    int level;
> >> +    int threads;
> >> +    int dthreads;
> >> +};
> > 
> > Actually, I think the names should correspond to the names used by QEMU
> > to avoid some confusion.
> 
> Ouch, this reveals some more misdesigned stuff. Look, I put qemuMonitorMigrationParameters
> into qemuMigrationCompression which is a kind of reverse aggregation. I see two
> options to make things the proper way here.
> 
> 1. Rename qemuMonitorMigrationParameters, qemuMonitorSetMigrationParameters etc
> to add compression to the naming somehow. If actual qemu command will one
> day be extended to support parameters from different category - well we
> just add another json monitor command that will get/set the new subset
> of parametes.
> 
> 2. Rework code so that we will aggregate all migration parameters into
> qemuMonitorSetMigrationParameters value and call actual qemu command
> only once. This way either we pack all compression values
> into one structure and have code that fills migration parameters value 
> appropriately or we stop keeping compression related parameters
> in qemuMigrationCompression. I don't like any of these.
> 
> So i prefer the first option.

Originally, I inclined to some form of option 2, but now I agree option
1 would be better. At least until we have other migration parameters,
but we can rethink the design if needed then.

Jirka


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