Re: [Qemu-devel] [RFC PATCH V3 1/4] colo-compare: introduce colo compare initlization

[Jason Wang]

On 04/28/2016 03:55 PM, Zhang Chen wrote:
>
>
> On 04/28/2016 03:16 PM, Jason Wang wrote:
>>
>> On 04/28/2016 02:53 PM, Jason Wang wrote:
>>> +static void compare_set_outdev(Object *obj, const char *value,
>>> Error **errp)
>>>> +{
>>>> +    CompareState *s = COLO_COMPARE(obj);
>>>> +
>>>> +    g_free(s->outdev);
>>>> +    s->outdev = g_strdup(value);
>>>> +}
>>>> +
>>>> +/*
>>>> + * called from the main thread on the primary
>>>> + * to setup colo-compare.
>>>> + */
>>>> +static void colo_compare_complete(UserCreatable *uc, Error **errp)
>>>> +{
>>>> +    CompareState *s = COLO_COMPARE(uc);
>>>> +
>>>> +    if (!s->pri_indev || !s->sec_indev || !s->outdev) {
>>>> +        error_setg(errp, "colo compare needs 'primary_in' ,"
>>>> +                   "'secondary_in','outdev' property set");
>>>> +        return;
>>>> +    } else if (!strcmp(s->pri_indev, s->outdev) ||
>>>> +               !strcmp(s->sec_indev, s->outdev) ||
>>>> +               !strcmp(s->pri_indev, s->sec_indev)) {
>>>> +        error_setg(errp, "'indev' and 'outdev' could not be same "
>>>> +                   "for compare module");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    s->chr_pri_in = qemu_chr_find(s->pri_indev);
>>>> +    if (s->chr_pri_in == NULL) {
>>>> +        error_setg(errp, "Primary IN Device '%s' not found",
>>>> +                   s->pri_indev);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    s->chr_sec_in = qemu_chr_find(s->sec_indev);
>>>> +    if (s->chr_sec_in == NULL) {
>>>> +        error_setg(errp, "Secondary IN Device '%s' not found",
>>>> +                   s->sec_indev);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    s->chr_out = qemu_chr_find(s->outdev);
>>>> +    if (s->chr_out == NULL) {
>>>> +        error_setg(errp, "OUT Device '%s' not found", s->outdev);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    qemu_chr_fe_claim_no_fail(s->chr_pri_in);
>>>> +    qemu_chr_add_handlers(s->chr_pri_in, compare_chr_can_read,
>>>> +                          compare_pri_chr_in, NULL, s);
>>>> +
>>>> +    qemu_chr_fe_claim_no_fail(s->chr_sec_in);
>>>> +    qemu_chr_add_handlers(s->chr_sec_in, compare_chr_can_read,
>>>> +                          compare_sec_chr_in, NULL, s);
>>>> +
>> Btw, what's the reason of handling this in main loop? I thought it would
>> be better to do this in colo thread? Otherwise, you need lots of extra
>> synchronizations?
>
> Do you mean we should start/stop/do checkpoint it by colo-frame?

I mean we probably want to handle pri_in and sec_in in colo compare
thread. Through this way, there's no need for extra synchronization with
main loop.

>
>>
>>>> +    qemu_chr_fe_claim_no_fail(s->chr_out);
>>>> +    QTAILQ_INSERT_TAIL(&net_compares, s, next);
>>>> +
>>>> +    return;
>>>> +}
>>
>>
>> .
>>
>