Skip to content

Commit

Permalink
Merge remote-tracking branch 'github-origin/stable-v2.0' into stable-…
Browse files Browse the repository at this point in the history
…v2.0
  • Loading branch information
SergeiShtepa committed Oct 31, 2023
2 parents 10797c0 + a34b36e commit 6ec36bf
Show file tree
Hide file tree
Showing 11 changed files with 14 additions and 11 deletions.
6 changes: 3 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ jobs:
<<: *shared
docker:
- image: library/ubuntu:jammy
ubuntu2210:
ubuntu2310:
<<: *shared
docker:
- image: library/ubuntu:kinetic
- image: library/ubuntu:mantic

workflows:
build-deb:
Expand All @@ -74,4 +74,4 @@ workflows:
- debian12
- ubuntu2004
- ubuntu2204
- ubuntu2210
- ubuntu2310
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/Bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ body:
- type: textarea
id: additional-info
attributes:
label: Additional informations
label: Additional information
description: Add any additional information related to the issue here.


2 changes: 1 addition & 1 deletion .github/workflows/Linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
fail-fast: false

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Install packages required
run: |
Expand Down
5 changes: 3 additions & 2 deletions .github/workflows/build-other-archs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,19 @@ on:
jobs:
build_job:
# The host should always be linux
runs-on: ubuntu-20.04
runs-on: ubuntu-22.04
name: ${{ matrix.arch }}

strategy:
fail-fast: false
matrix:
include:
- arch: aarch64
- arch: ppc64le
- arch: s390x
- arch: armv7
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- uses: uraimo/run-on-arch-action@v2
name: Build
id: build
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ kernel documentation, comments in source code etc...

Testing is important to have a quality software, see related part in [README](README.md#tests)
and the [README](https://github.com/veeam/blksnap/blob/master/doc/README-upstream-kernel.md) related to upstream kernel integration
for some informations.
for some information.

## Contributing to Source Code

Expand Down
2 changes: 1 addition & 1 deletion doc/blksnap.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ A change tracker map is created for each block device. One byte of this map corr

The byte of the change tracking map stores a number from 0 to 255. This is the sequence number of the snapshot for which there have been changes in the block since the snapshot was taken. Each time a snapshot is taken, the number of the current snapshot increases by one. This number is written to the cell of the change tracking map when writing to the block. Thus, knowing the number of one of the previous snapshots and the number of the last one, we can determine from the change tracking map which blocks have been changed. When the number of the current change has reached the maximum allowed value for the map of 255, when creating the next snapshot, the change tracking map is reset to zero, and the number of the current snapshot is assigned the value 1. The tracker of changes is reset and a new UUID — a unique identifier of the generation of snapshots — is generated. The snapshot generation identifier allows to identify that a change tracking reset has been performed.

The change map has two copies. One is active, and it tracks the current changes on the block device. The second one is available for reading while the snapshot is being held, and it contains the history of changes that occured before the snapshot was taken. Copies are synchronized at the moment of taking a snapshot. After the snapshot is released, a second copy of the map is not needed, but it is not released, so as not to allocate memory for it again the next time the snapshot is created.
The change map has two copies. One is active, and it tracks the current changes on the block device. The second one is available for reading while the snapshot is being held, and it contains the history of changes that occurred before the snapshot was taken. Copies are synchronized at the moment of taking a snapshot. After the snapshot is released, a second copy of the map is not needed, but it is not released, so as not to allocate memory for it again the next time the snapshot is created.

### Copy-on-write
Data is copied in blocks, or rather in chunks. The term "chunk" is used not to confuse it with change tracker blocks and I/O blocks. In addition, the "chunk" in the blksnap module means about the same as the "chunk" in the dm-snap module.
Expand Down
1 change: 1 addition & 0 deletions include/blksnap/TrackerCtl.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* flexibility. Uses structures that are directly passed to the kernel module.
*/

#include <stdint.h>
#include <string>
#include <uuid/uuid.h>
#include <vector>
Expand Down
2 changes: 1 addition & 1 deletion include/linux/blksnap.h
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ struct blksnap_snapshot_event {
* While holding the snapshot, the kernel module can transmit information about
* changes in its state in the form of events to the user level.
* It is very important to receive these events as quickly as possible, so the
* user's thread is in the state of interruptable sleep.
* user's thread is in the state of interruptible sleep.
*
* Return: 0 if succeeded, negative errno otherwise.
*/
Expand Down
1 change: 1 addition & 0 deletions lib/blksnap/TrackerCtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <blksnap/TrackerCtl.h>
#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/ioctl.h>
Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion tools/blksnap/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ class SnapshotAddArgsProc : public IArgsProc
SnapshotAddArgsProc()
: IArgsProc()
{
m_usage = std::string("Add device fo snapshot.");
m_usage = std::string("Add device for snapshot.");
m_desc.add_options()
("device,d", po::value<std::string>(), "Device name.")
("id,i", po::value<std::string>(), "Snapshot uuid.");
Expand Down

3 comments on commit 6ec36bf

@Fantu
Copy link
Contributor

@Fantu Fantu commented on 6ec36bf Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SergeiShtepa hi, with these merge of local branch instead rebase (if github repo have newer commits) before push makes it difficult to see the changes
from the github site alone I was unable to see and understand which commits you added and I should instead do some comparative commands on git where I have the repository to see the differences (in the merge commit on the site it only shows the changes of the merge did on the local copy)
If it's not a problem could you please rebase instead merge (updating local repo)? at least in cases that apply cleanly without manual modifications

@SergeiShtepa
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.
If "merge" creates a problem, then I will try to do "Rebase".

@Fantu
Copy link
Contributor

@Fantu Fantu commented on 6ec36bf Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SergeiShtepa thanks, and sorry if I stress you out
I try to explain in detail my idea, which seems to me convenient and similar to the management of most of the projects I've seen.
Then obviously you can do whatever you want, I don't want try to force you.

Merge for PR from forks and other branches present in the public git are common ones, you can keep track of the origin and the merge commit gives the diff of that change visible quickly and easily even from the github site.

Merge for update local copy, in case of merged PR or push of other developers after latest pull, do a merge commit with diff that contain changes done on github repository from previous pull.
Therefore technically it does not add the display of a "new" work, contrary to PR merges of a branch on which specific work was done, perhaps a long one while the main branch proceeded with something else and then in the end a merge was done with conflict resolution.
These merge of update of local copy, on github site in page like this show diff of other merges and commits already done but not the new changes pushed.
A workaround to see the changes done by these merge on local copy is with software like DAG and for example on this I saw only one commit (10797c0) was added (and I didn't understand it from a look at the history on github site): https://ibb.co/41kbLKy
So my suggest is done rebase for update local copy (if new commits was already done but not pushed) at least when this can be done without conflict to solves manually (in some cases of conflict to solve manually merge is easier than rebase so ok use merge)

As I have seen over the years, it can help to always do a pull before starting some new work, this helps to reduce the necessary local merges/rebases.
Before pulling and starting a new work, it can also be useful to look at the existing PRs and possibly merge them (except they do not require a more accurate review; or don't correspond to current development plans, branch etc...). For example, in some cases, having not checked the PRs carefully, I unfortunately even wasted time preparing something that had already been done by others.
I hope I have given useful information and advice, written in an understandable way (I have difficulty with this) and have not wasted your time or annoyed you.

Please sign in to comment.