Re: [Qemu-devel] [PATCH V8 15/17] filter-rewriter: handle checkpoint and failover event

[Zhang Chen]

On Mon, Jun 4, 2018 at 3:42 PM, Jason Wang <address@hidden> wrote:

>
>
> On 2018年06月03日 13:05, Zhang Chen wrote:
>
>> After one round of checkpoint, the states between PVM and SVM
>> become consistent, so it is unnecessary to adjust the sequence
>> of net packets for old connections, besides, while failover
>> happens, filter-rewriter needs to check if it still needs to
>> adjust sequence of net packets.
>>
>> Signed-off-by: zhanghailiang <address@hidden>
>> Signed-off-by: Zhang Chen <address@hidden>
>> ---
>>   migration/colo.c      | 13 +++++++++++++
>>   net/filter-rewriter.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 53 insertions(+)
>>
>> diff --git a/migration/colo.c b/migration/colo.c
>> index 442471e088..0bff21d9e5 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -31,6 +31,7 @@
>>   #include "qapi/qapi-events-migration.h"
>>   #include "qapi/qmp/qerror.h"
>>   #include "sysemu/cpus.h"
>> +#include "net/filter.h"
>>     static bool vmstate_loading;
>>   static Notifier packets_compare_notifier;
>> @@ -82,6 +83,11 @@ static void secondary_vm_do_failover(void)
>>       if (local_err) {
>>           error_report_err(local_err);
>>       }
>> +    /* Notify all filters of all NIC to do checkpoint */
>> +    colo_notify_filters_event(COLO_EVENT_FAILOVER, &local_err);
>> +    if (local_err) {
>> +        error_report_err(local_err);
>> +    }
>>         if (!autostart) {
>>           error_report("\"-S\" qemu option will be ignored in secondary
>> side");
>> @@ -800,6 +806,13 @@ void *colo_process_incoming_thread(void *opaque)
>>               goto out;
>>           }
>>   +        /* Notify all filters of all NIC to do checkpoint */
>> +        colo_notify_filters_event(COLO_EVENT_CHECKPOINT, &local_err);
>> +        if (local_err) {
>> +            qemu_mutex_unlock_iothread();
>> +            goto out;
>> +        }
>> +
>>
>
> I think the above should belong to another patch.
>
>
>
Yes, I will relocate above code.



>           vmstate_loading = false;
>>           vm_start();
>>           trace_colo_vm_state_change("stop", "run");
>> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
>> index 0909a9a8af..f3c306cc89 100644
>> --- a/net/filter-rewriter.c
>> +++ b/net/filter-rewriter.c
>> @@ -20,6 +20,8 @@
>>   #include "qemu/main-loop.h"
>>   #include "qemu/iov.h"
>>   #include "net/checksum.h"
>> +#include "net/colo.h"
>> +#include "migration/colo.h"
>>     #define FILTER_COLO_REWRITER(obj) \
>>       OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
>> @@ -277,6 +279,43 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState
>> *nf,
>>       return 0;
>>   }
>>   +static void reset_seq_offset(gpointer key, gpointer value, gpointer
>> user_data)
>> +{
>> +    Connection *conn = (Connection *)value;
>> +
>> +    conn->offset = 0;
>> +}
>> +
>> +static gboolean offset_is_nonzero(gpointer key,
>> +                                  gpointer value,
>> +                                  gpointer user_data)
>> +{
>> +    Connection *conn = (Connection *)value;
>> +
>> +    return conn->offset ? true : false;
>> +}
>> +
>> +static void colo_rewriter_handle_event(NetFilterState *nf, int event,
>> +                                       Error **errp)
>> +{
>> +    RewriterState *rs = FILTER_COLO_REWRITER(nf);
>> +
>> +    switch (event) {
>> +    case COLO_EVENT_CHECKPOINT:
>> +        g_hash_table_foreach(rs->connection_track_table,
>> +                            reset_seq_offset, NULL);
>> +        break;
>> +    case COLO_EVENT_FAILOVER:
>> +        if (!g_hash_table_find(rs->connection_track_table,
>> +                              offset_is_nonzero, NULL)) {
>> +            object_property_set_str(OBJECT(nf), "off", "status", errp);
>> +        }
>>
>
> I may miss something but I think rewriter should not be turned off even
> after failover?
>
>
Yes, after failover, rewriter should maintain the connection that created
before failover and turned off for other connections.
I will fix it in next version.
Very thanks your comments Jason~~~


Thanks
Zhang Chen




> Thanks
>
>
> +        break;
>> +    default:
>> +        break;
>> +    }
>> +}
>> +
>>   static void colo_rewriter_cleanup(NetFilterState *nf)
>>   {
>>       RewriterState *s = FILTER_COLO_REWRITER(nf);
>> @@ -332,6 +371,7 @@ static void colo_rewriter_class_init(ObjectClass
>> *oc, void *data)
>>       nfc->setup = colo_rewriter_setup;
>>       nfc->cleanup = colo_rewriter_cleanup;
>>       nfc->receive_iov = colo_rewriter_receive_iov;
>> +    nfc->handle_event = colo_rewriter_handle_event;
>>   }
>>     static const TypeInfo colo_rewriter_info = {
>>
>
>